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

feat: #1179 bceid login #1225

Merged
merged 12 commits into from
Mar 19, 2024
Merged

feat: #1179 bceid login #1225

merged 12 commits into from
Mar 19, 2024

Conversation

MCatherine1994
Copy link
Contributor

@MCatherine1994 MCatherine1994 commented Mar 6, 2024

refs: #1179

  • Support business bceid login in FAM, use TEST-BUSINESSBCEID for local and dev environment, as we don't have any DEV business bceid for testing and playing around. To keep consistency we use TEST-IDIR for local and dev environment as well, so the same logout chain could work for both. Update the backend local cognito user id to use "test-idir", update backend test to use "test-idir" as well
  • Update the profile to display more information
Screen Shot 2024-03-18 at 10 12 14 AM Screen Shot 2024-03-18 at 10 25 23 AM
  • Admin management endpoint get_admin_user_access doesn't need to check access group in the token, delegated admin won't have any access in the token. That API will return the access for the login user. Created a new router guard method to return requester data without check the access role in the token

@basilv
Copy link
Collaborator

basilv commented Mar 12, 2024

@MCatherine1994 I didn't understand your comment "Note: Our logout chain in local and dev connects with the dev environment, so the logout for test business bceid will only log the user out from Cognito but still have the sitemider session. " - why wouldn't local/dev logout chain be configured for the test business BCeID environment? Is that because the dev keycloak instance is only pointing at dev BCeID, not test?

@MCatherine1994
Copy link
Contributor Author

@MCatherine1994 I didn't understand your comment "Note: Our logout chain in local and dev connects with the dev environment, so the logout for test business bceid will only log the user out from Cognito but still have the sitemider session. " - why wouldn't local/dev logout chain be configured for the test business BCeID environment? Is that because the dev keycloak instance is only pointing at dev BCeID, not test?

Hi Basil, that's just because for our local/dev environment, we suppose to use DEV-IDIR and DEV-BUSINESS-BCEID, and they work with the same DEV logout chain. But because we don't have dev bceid for testing, I setup to use DEV-IDIR and TEST-BUSINESS-BCEID for local/dev environment. So DEV-IDIR needs dev logout chain, but TEST-BUSINESS-BCEID needs test logout chain.
The Amplify just configured with one redirect logout url doesn't support both in the current code. And I feel it's fine so I didn't change the Amplify config. Because for TEST env, IDIR and BCEID will both connect with TEST, and same for PROD. So the issue is only in DEV.
Now I'm thinking maybe we can just use TEST-IDIR and TEST-BUSINESS-BCEID for local and dev, and config the test logout chain, so logout can work properly for both. And use DEV orTEST IDIR won't impact the login behaviour?

@basilv
Copy link
Collaborator

basilv commented Mar 12, 2024

@MCatherine1994 I didn't understand your comment "Note: Our logout chain in local and dev connects with the dev environment, so the logout for test business bceid will only log the user out from Cognito but still have the sitemider session. " - why wouldn't local/dev logout chain be configured for the test business BCeID environment? Is that because the dev keycloak instance is only pointing at dev BCeID, not test?

Hi Basil, that's just because for our local/dev environment, we suppose to use DEV-IDIR and DEV-BUSINESS-BCEID, and they work with the same DEV logout chain. But because we don't have dev bceid for testing, I setup to use DEV-IDIR and TEST-BUSINESS-BCEID for local/dev environment. So DEV-IDIR needs dev logout chain, but TEST-BUSINESS-BCEID needs test logout chain. The Amplify just configured with one redirect logout url doesn't support both in the current code. And I feel it's fine so I didn't change the Amplify config. Because for TEST env, IDIR and BCEID will both connect with TEST, and same for PROD. So the issue is only in DEV. Now I'm thinking maybe we can just use TEST-IDIR and TEST-BUSINESS-BCEID for local and dev, and config the test logout chain, so logout can work properly for both. And use DEV orTEST IDIR won't impact the login behaviour?

This makes sense, just as long as this is documented somewhere in the dev configuration.

basilv
basilv previously approved these changes Mar 13, 2024
@MCatherine1994 MCatherine1994 marked this pull request as draft March 13, 2024 16:36
Copy link

sonarcloud bot commented Mar 19, 2024

Quality Gate Passed Quality Gate passed for 'nr-forests-access-management_admin'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@MCatherine1994 MCatherine1994 merged commit eb815a0 into main Mar 19, 2024
16 checks passed
@MCatherine1994 MCatherine1994 deleted the feat/1179-bceid-login branch March 19, 2024 17:38
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.

2 participants