-
Notifications
You must be signed in to change notification settings - Fork 546
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
Improve authenticator management #6290
base: master
Are you sure you want to change the base?
Improve authenticator management #6290
Conversation
62bf5b7
to
682372d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6290 +/- ##
=========================================
Coverage ? 45.90%
Complexity ? 14252
=========================================
Files ? 1655
Lines ? 101268
Branches ? 17576
=========================================
Hits ? 46491
Misses ? 48055
Partials ? 6722
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
PR builder started |
PR builder completed |
PR builder started |
PR builder completed |
...c/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java
Outdated
Show resolved
Hide resolved
throws AuthenticatorMgtException { | ||
|
||
List<LocalAuthenticatorConfig> configList = new ArrayList<>(getAllUserDefinedLocalAuthenticators(tenantDomain)); | ||
configList.addAll(localAuthenticators); |
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.
At a given point of time looks like this won't return all system defined authenticators. This is because system defined authenticators are provided an input to the service.
This is fine for the moment as that is how this service is defined
.../carbon/identity/application/authentication/framework/config/model/graph/JsGraphBuilder.java
Outdated
Show resolved
Hide resolved
...2/carbon/identity/application/authentication/framework/ApplicationAuthenticationService.java
Outdated
Show resolved
Hide resolved
a59c5b0
to
11e2dbb
Compare
The Sonar reporting following issues, which are false positive.
|
11e2dbb
to
dd2bc07
Compare
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
Issue:
With custom authentication extension feature, there will be two type of local authenticators, system defined local authenticators and user defined local authenticator. The current existing method for retrieving all local authenticators only consider system defined local authenticators and not returning returning user defined authenticators.
The code logic of those methods cannot be updated to retrieve the user defined local authenticators as well, as it required tenant domain, but those methods does not get tenant domain as parameter. Therefore, those methods will be depreciated and introduce new methods to retrieve both system and local authenticators.