-
Notifications
You must be signed in to change notification settings - Fork 263
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
Add Custom Authentication Edit Page #7363
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7363 +/- ##
=======================================
Coverage 36.32% 36.32%
=======================================
Files 42 42
Lines 903 903
Branches 205 205
=======================================
Hits 328 328
Misses 575 575
Flags with carried forward coverage won't be shown. Click here to find out more. |
features/admin.connections.v1/components/create/authenticator-create-wizard-factory.tsx
Outdated
Show resolved
Hide resolved
@@ -106,6 +108,9 @@ export const AuthenticatorCreateWizardFactory: FC<AuthenticatorCreateWizardFacto | |||
...rest | |||
} = props; | |||
|
|||
const customAuthFeatureConfig: FeatureAccessConfigInterface = useSelector((state: AppState) => |
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.
const customAuthFeatureConfig: FeatureAccessConfigInterface = useSelector((state: AppState) => | |
const identityProviderFeatureConfig: FeatureAccessConfigInterface = useSelector((state: AppState) => |
@@ -330,6 +337,27 @@ export const AuthenticatorCreateWizardFactory: FC<AuthenticatorCreateWizardFacto | |||
/> | |||
); | |||
|
|||
case CommonAuthenticatorConstants.CONNECTION_TEMPLATE_IDS.CUSTOM_AUTHENTICATION: | |||
if (isFeatureEnabled(customAuthFeatureConfig, "identityProviders.customAuthentication")) { |
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.
Let's define the identityProviders.customAuthentication
as a feature identifier in /constants and refer it here.
This is done to organize the feature identifiers for each feature in a central location.
Eg:
if (isFeatureEnabled(customAuthFeatureConfig, "identityProviders.customAuthentication")) { | |
if (isFeatureEnabled( | |
customAuthFeatureConfig, ConnectionUIConstants.FEATURE_DICTIONARY.CUSTOM_AUTHENTICATION)) { |
Ref. definition of feature dictionary object: https://github.com/wso2/identity-apps/blob/master/features/admin.groups.v1/constants/group-constants.ts#L40
features/admin.connections.v1/components/create/custom-authentication-create-wizard.scss
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/create/custom-authentication-create-wizard.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/create/custom-authentication-create-wizard.tsx
Outdated
Show resolved
Hide resolved
features/admin.connections.v1/components/create/custom-authentication-create-wizard.tsx
Show resolved
Hide resolved
features/admin.connections.v1/components/create/custom-authentication-create-wizard.tsx
Show resolved
Hide resolved
); | ||
} | ||
|
||
|
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.
data-componentid={ `${ _componentId }-idp-image` } | ||
maxLength={ | ||
ConnectionUIConstants | ||
.GENERAL_FORM_CONSTRAINTS.IMAGE_URL_MAX_LENGTH as number |
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.
Do we need casting here?
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.
We have followed the same approach used in other components in the connections page.
It appears that the GENERAL_FORM_CONSTRAINTS record's value could be a union of string or number and here we are explicitly type casting it to string. But this seems like an additional overhead since we using typescript.
ConnectionUIConstants | ||
.GENERAL_FORM_CONSTRAINTS.IMAGE_URL_MIN_LENGTH as number | ||
} | ||
hint="Logo to display in login pages." |
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.
Let's use i18n for string literals
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.
Ack
data-componentid={ `${ _componentId }-idp-description` } | ||
maxLength={ ConnectionUIConstants.IDP_NAME_LENGTH.max } | ||
minLength={ ConnectionUIConstants.IDP_NAME_LENGTH.min } | ||
hint="A text description of the authenticator." |
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.
Let's use i18n for string literals. Also let's get the UI text reviewed from doc team.
hint="A text description of the authenticator." | |
hint="A brief description of the authenticator." |
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.
Ack
🦋 Changeset detectedThe changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. |
Purpose
This PR updates the edit page of the custom authentication feature. This also adds API integrations.
Note: This is needs to be updated once #7336 is merged this PR depends on the content of the above.
Related Issues
Related PRs (To be merged before)
Checklist
Security checks