-
Notifications
You must be signed in to change notification settings - Fork 112
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.
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
Refactor Login Page Creation in initiateAuthenticationRequest
Method
#177
Conversation
String loginPage = prepareLoginPage(request, context); | ||
response.sendRedirect(loginPage); | ||
if (LoggerUtils.isDiagnosticLogsEnabled()) { | ||
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need build the diagnostic builder when refactoring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is initially created in the refactored method, prepareLoginPage()
. Since we are using diagnostic logs outside of the refactored method, we need to create it again.
As an alternative, we can create the diagnosticLogBuilder
at the top of the initiateAuthenticationRequest()
method and pass it as a param to prepareLoginPage()
(only if diagnostic logs are enabled, otherwise a null value will be passed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @return Extracted scopes as a String. | ||
*/ | ||
protected String extractScopesFromURL(String url) throws UnsupportedEncodingException { | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with 1bfe9d9
PR builder started |
PR builder completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/8579215697
Purpose
initiateAuthenticationRequest()
method into protected method.Related Issues