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.

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 authentication name validation. #6288

Conversation

Thisara-Welmilla
Copy link
Contributor

@Thisara-Welmilla Thisara-Welmilla commented Jan 16, 2025

Issue:

Following improvements need to be done to the custom authentication management.

  1. Addressing Naming Conflicts for User-Defined and future System-Defined Authenticators
    Currently, for user-defined authenticators, it is checking for any conflict with system-defined names making authenticator name unique.
    The reqirement is to have unique identifier for the amr values. However, with the implementation so far, if a custom authenticator is created with the name whatsappAuthenticator and we later introduce a system authenticator with the same name, it would result in a conflict. There improve authenticator name regex validation to check whether authenticator name start with custom- prefix with atleast three charactors.
  2. In addtion to above validate authenticator name uniqueness across both local and federared authenticator names.
  3. Improve display name validation to check whether it has atleast three charactors.

@Thisara-Welmilla Thisara-Welmilla force-pushed the improve-authenticator-name-validation branch 2 times, most recently from 956d679 to 5bbf343 Compare January 16, 2025 06:42
@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12850786254

Copy link

codecov bot commented Jan 19, 2025

Codecov Report

Attention: Patch coverage is 72.09302% with 12 lines in your changes missing coverage. Please review.

Project coverage is 46.20%. Comparing base (368134f) to head (f553489).
Report is 65 commits behind head on master.

Files with missing lines Patch % Lines
...cation/common/ApplicationAuthenticatorService.java 57.14% 4 Missing and 2 partials ⚠️
...ommon/dao/impl/AuthenticatorManagementDAOImpl.java 80.00% 2 Missing ⚠️
...common/dao/impl/AuthenticatorManagementFacade.java 60.00% 2 Missing ⚠️
...g/wso2/carbon/idp/mgt/IdentityProviderManager.java 60.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6288      +/-   ##
============================================
+ Coverage     45.91%   46.20%   +0.28%     
+ Complexity    14481    14454      -27     
============================================
  Files          1672     1672              
  Lines        103141   102230     -911     
  Branches      18010    17876     -134     
============================================
- Hits          47360    47238     -122     
+ Misses        48970    48220     -750     
+ Partials       6811     6772      -39     
Flag Coverage Δ
unit 29.30% <72.09%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12850786254
Status: failure

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12854554991

@Thisara-Welmilla Thisara-Welmilla force-pushed the improve-authenticator-name-validation branch from 1066f2a to 67dbed1 Compare January 19, 2025 14:59
@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12854554991
Status: failure

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12861003816

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12861003816
Status: failure

@Thisara-Welmilla
Copy link
Contributor Author

PR builder completed Link: https://github.com/wso2/product-is/actions/runs/12861003816 Status: failure

The failing integration tests are due to error messages changes, with the wso2/product-is#22265 they will be addressed.

@Thisara-Welmilla Thisara-Welmilla merged commit 91da870 into wso2:master Jan 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants