Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Login Page Creation in initiateAuthenticationRequest Method #177

Merged
merged 4 commits into from
Apr 6, 2024
Merged

Refactor Login Page Creation in initiateAuthenticationRequest Method #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fee04f6
Refactor login page creation in initiateAuthenticationRequest method.
dhaura Apr 5, 2024
784af7d
Resolve diagnostic log builders.
dhaura Apr 5, 2024
8598f8b
Refactor diagnostic log builders.
dhaura Apr 5, 2024
1bfe9d9
Improve extractScopesFromURL() method.
dhaura Apr 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...a/org/wso2/carbon/identity/application/authenticator/oidc/OIDCAuthenticatorConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ private OIDCAuthenticatorConstants() {
public static final String SCOPE_PARAM_SUFFIX = "_scope_param";
public static final String REDIRECTION_PROMPT = "REDIRECTION_PROMPT";
public static final String SCOPE = "scope";
public static final String QUESTION_SIGN = "\\?";
public static final String AMPERSAND_SIGN = "&";
public static final String EQUAL_SIGN = "=";

/**
* This class holds the constants related to authenticator configuration parameters.
Expand Down
92 changes: 67 additions & 25 deletions ...a/org/wso2/carbon/identity/application/authenticator/oidc/OpenIDConnectAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, WSO2 LLC. (http://www.wso2.com).
* Copyright (c) 2013-2024, WSO2 LLC. (http://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -93,6 +93,7 @@
import java.io.UnsupportedEncodingException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
Expand Down Expand Up @@ -449,9 +450,48 @@ protected Map<ClaimMapping, String> getSubjectAttributes(OAuthClientResponse tok
protected void initiateAuthenticationRequest(HttpServletRequest request, HttpServletResponse response,
AuthenticationContext context) throws AuthenticationFailedException {

try {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = null;
if (LoggerUtils.isDiagnosticLogsEnabled() && context.getAuthenticatorProperties() != null) {
diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder(
getComponentId(), INITIATE_OUTBOUND_AUTH_REQUEST);
diagnosticLogBuilder.logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION)
.resultStatus(DiagnosticLog.ResultStatus.SUCCESS)
.inputParam(LogConstants.InputKeys.STEP, context.getCurrentStep())
.inputParam("authenticator properties", context.getAuthenticatorProperties().keySet())
.inputParam(LogConstants.InputKeys.IDP, context.getExternalIdP().getIdPName())
.inputParams(getApplicationDetails(context));
}

String loginPage = prepareLoginPage(request, context);
response.sendRedirect(loginPage);
if (LoggerUtils.isDiagnosticLogsEnabled() && diagnosticLogBuilder != null) {
String scopes = extractScopesFromURL(loginPage);
if (StringUtils.isNotEmpty(scopes)) {
diagnosticLogBuilder.inputParam("scopes", scopes);
}
diagnosticLogBuilder.resultMessage("Redirecting to the federated IDP login page.");
LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
}
} catch (IOException e) {
throw new AuthenticationFailedException(ErrorMessages.IO_ERROR.getCode(), e.getMessage(), e);
}
}

/**
* Prepare the login page needed for initiating authentication request.
*
* @param request Http Servlet Request.
* @param context Authentication Context of the flow.
* @return Login page needed for initiating authentication request.
*/
protected String prepareLoginPage(HttpServletRequest request, AuthenticationContext context)
throws AuthenticationFailedException {

try {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder(
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder =
new DiagnosticLog.DiagnosticLogBuilder(
getComponentId(), INITIATE_OUTBOUND_AUTH_REQUEST);
diagnosticLogBuilder.resultMessage("Initiate outbound OIDC authentication request.")
.logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION)
Expand All @@ -463,17 +503,6 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer
}
Map<String, String> authenticatorProperties = context.getAuthenticatorProperties();
if (authenticatorProperties != null) {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = null;
if (LoggerUtils.isDiagnosticLogsEnabled()) {
diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder(getComponentId(),
INITIATE_OUTBOUND_AUTH_REQUEST);
diagnosticLogBuilder.logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION)
.resultStatus(DiagnosticLog.ResultStatus.SUCCESS)
.inputParam(LogConstants.InputKeys.STEP, context.getCurrentStep())
.inputParam("authenticator properties", authenticatorProperties.keySet())
.inputParam(LogConstants.InputKeys.IDP, context.getExternalIdP().getIdPName())
.inputParams(getApplicationDetails(context));
}
String clientId = authenticatorProperties.get(OIDCAuthenticatorConstants.CLIENT_ID);
String authorizationEP = getOIDCAuthzEndpoint(authenticatorProperties);
String callbackurl = getCallbackUrl(authenticatorProperties);
Expand Down Expand Up @@ -502,9 +531,6 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer

String queryString = getQueryString(authenticatorProperties);
if (StringUtils.isNotBlank(scopes)) {
if (LoggerUtils.isDiagnosticLogsEnabled() && diagnosticLogBuilder != null) {
diagnosticLogBuilder.inputParam("scopes", scopes);
}
queryString += "&scope=" + scopes;
}
queryString = interpretQueryString(context, queryString, request.getParameterMap());
Expand Down Expand Up @@ -567,11 +593,7 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer
}
}
context.setProperty(OIDCAuthenticatorConstants.AUTHENTICATOR_NAME + REDIRECT_URL_SUFFIX, loginPage);
response.sendRedirect(loginPage);
if (LoggerUtils.isDiagnosticLogsEnabled() && diagnosticLogBuilder != null) {
diagnosticLogBuilder.resultMessage("Redirecting to the federated IDP login page.");
LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
}
return loginPage;
} else {
if (LOG.isDebugEnabled()) {
LOG.debug(ErrorMessages.RETRIEVING_AUTHENTICATOR_PROPERTIES_FAILED.getMessage());
Expand All @@ -590,13 +612,10 @@ protected void initiateAuthenticationRequest(HttpServletRequest request, HttpSer

throw new AuthenticationFailedException(ErrorMessages.BUILDING_AUTHORIZATION_CODE_REQUEST_FAILED.getCode(),
e.getMessage(), e);
} catch (IOException e) {
throw new AuthenticationFailedException(ErrorMessages.IO_ERROR.getCode(), e.getMessage(), e);
} catch (OAuthSystemException e) {
throw new AuthenticationFailedException(ErrorMessages.BUILDING_AUTHORIZATION_CODE_REQUEST_FAILED.getCode(),
e.getMessage(), e);
}
return;
}

/**
Expand Down Expand Up @@ -2038,7 +2057,7 @@ private AuthenticatorFlowStatus processLogout(HttpServletRequest request, HttpSe
* @param context Authentication context.
* @return Map of application details.
*/
private Map<String, String> getApplicationDetails(AuthenticationContext context) {
protected Map<String, String> getApplicationDetails(AuthenticationContext context) {

Map<String, String> applicationDetailsMap = new HashMap<>();
FrameworkUtils.getApplicationResourceId(context).ifPresent(applicationId ->
Expand All @@ -2049,6 +2068,29 @@ private Map<String, String> getApplicationDetails(AuthenticationContext context)
return applicationDetailsMap;
}

/**
* Extract query param scopes from a given url.
*
* @param url Given url.
* @return Extracted scopes as a String.
*/
protected String extractScopesFromURL(String url) throws UnsupportedEncodingException {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we extract the query params first from URL and them execute the next logic on top of that?

If the scope is the first query param (<URL>?scope=a b c&...), I think the following logic does not recognize the scopes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed with 1bfe9d9

if (StringUtils.isNotBlank(url)) {
String[] splitUrl = url.split(OIDCAuthenticatorConstants.QUESTION_SIGN, 2);
if (splitUrl.length == 2) {
String[] params = splitUrl[1].split(OIDCAuthenticatorConstants.AMPERSAND_SIGN);
for (String param : params) {
String[] keyValue = param.split(OIDCAuthenticatorConstants.EQUAL_SIGN, 2);
if (keyValue.length == 2 && OAuthConstants.OAuth20Params.SCOPE.equals(keyValue[0])) {
return URLDecoder.decode(keyValue[1], FrameworkUtils.UTF_8);
}
}
}
}
return StringUtils.EMPTY;
}

private static List<String> getUserAttributeClaimMappingList(AuthenticatedUser authenticatedUser) {

return authenticatedUser.getUserAttributes().keySet().stream()
Expand Down
Loading