EP-2362 - Okta #1080

dr-bizz · 2023-12-11T19:56:48Z

Description

Replacing our login/logout functionality with Okta. This PR allows users to login and create an account with Okta.

Sign in

To sign, it's very much like other applications where the user gets redirected to Okta and brought back to the redirect URL.
We no longer maintain login/session data on our local storage or session storage. This is now stored via Okta.

Register

The user enters their email, first name and last name. On submit, we do a post request to create an account on Okta while keeping the user on the Give site.
Once that is complete, they go into the next phase of activating their email. Once activated it will automatically update the modal to send the user to the next step.

Jira

wrandall22 · 2023-12-11T21:09:13Z

Doesn't seem to like import { afterEach } from 'node:test'

wrandall22 · 2023-12-11T21:18:49Z

src/app/productConfig/productConfigForm/productConfigForm.component.js

@@ -388,6 +389,11 @@ class ProductConfigFormController {
      this.$window.location = url
    }
  }
+
+  shouldShowForcedUserToLogoutError() {


Tell me more about this new function

On adding an item to the cart, we check to see if the user has a public role, is logged into Okta and has no items in their account. If so, we automatically force the user to logout.

This was happening before, but when it was logging out, it didn't require a refresh. Okta requires a refresh since we send the user to the Okta site to clear Okta's session data and then return to the Give site.

We handle this here src/common/services/api/cart.service.js - Lines 164 - 181

On the sessionService.oktaSignOut we pass a prop which tells session data to store a prop called forcedUserToLogout. We also ensure Okta returns the user to the same page.
src/common/services/session/session.service.js Lines 317 - 322

Onsrc/app/productConfig/productConfigForm/productConfigForm.component.js, we watch for the forcedUserToLogout session data and show the warning to the user if present.

src/common/components/signInForm/signInForm.tpl.html

src/common/components/signUpActivationModal/signUpActivationModal.tpl.html

wrandall22 · 2023-12-20T20:54:30Z

the *cloud and nonprod environments are not in use anymore and can just be deleted, with the exception of devcloud.

src/common/services/session/session.service.js

wrandall22 · 2024-01-11T13:32:48Z

Not sure if the latest has been merged to stage yet or not, but something is still sending me to the home page when hitting branded checkout. You can test by going to https://give-stage2.cru.org/branded-checkout.html

wrandall22 · 2024-01-12T14:55:19Z

Here is a test branded checkout page that is not on give itself, we'll need to make sure the Okta logout works on pages like this as well.

wrandall22

Working through the files again to try and find anything we might have missed. Haven't gotten through it all, but want this feedback at least to get through.

src/app/profile/payment-methods/payment-methods.component.js

src/app/signIn/signIn.spec.js

src/common/components/signUpActivationModal/signUpActivationModal.tpl.html

src/common/components/signInForm/signInForm.tpl.html

src/common/components/contactInfo/contactInfo.tpl.html

src/common/components/registerAccountModal/registerAccountModal.tpl.html

wrandall22

Got through a few more.

src/common/components/registerAccountModal/registerAccountModal.component.js

wrandall22 · 2024-01-19T19:35:39Z

src/common/components/registerAccountModal/registerAccountModal.component.js

+    if (angular.isDefined(this.verificationServiceSubscription)) this.verificationServiceSubscription.unsubscribe()
+    this.verificationServiceSubscription = this.verificationService.postDonorMatches().subscribe({
+      next: () => { this.stateChanged('user-match') }, // Donor match success, Proceed to step 5.
+      error: () => { this.onCancel() } // Donor Match failed, Register Account workflow failed


I wonder if we should handle timeouts (503 response) differently. It currently brings the donor back to the home page with no indication that something went wrong. Probably out of scope for this PR since it is handling it the same way as before.

src/common/components/signInForm/signInForm.tpl.html

src/common/components/signUpModal/signUpModal.component.spec.js

wrandall22

Got through all the files, hopefully caught all the interesting things.

src/app/thankYou/accountBenefits/accountBenefits.component.js

src/common/components/signUpModal/signUpModal.tpl.html

src/common/components/userMatchModal/userMatchModal.component.js

src/common/components/userMatchModal/userMatchModal.component.spec.js

src/common/services/session/session.service.spec.js

src/common/services/session/sessionModal.service.js

src/common/services/session/sessionModal.service.spec.js

src/common/services/session/session.service.js

src/app/thankYou/accountBenefits/accountBenefits.component.spec.js

dr-bizz

I left a few comments and recommendations.

We should also add a index.page.ts which forwards /settings -> /settings/preferences/

dr-bizz · 2024-02-28T13:02:26Z

src/app/designationEditor/designationEditor.component.js

+    this.sessionHandleOktaRedirectService.errorMessageSubject.subscribe((errorMessage) => {
+      this.errorMessage = errorMessage
+    })


Logging the err to the this.errorMessage variable was in the original version of this file, so I've added it, but nowhere uses errorMessage so we can take it out unless you think we could use it in the future.

I'm not seeing this in the master branch. If it fails, it redirects to the home page. However, I do see $ctrl.errorMessage being used on the sign in form. Would that show this error message?

dr-bizz · 2024-02-28T13:18:34Z

src/app/profile/payment-methods/payment-methods.component.js

+    this.sessionHandleOktaRedirectService.errorMessageSubject.subscribe((errorMessage) => {
+      this.errorMessage = errorMessage
+    })


Here again with the errorMessage I've added it in, but since we don't use errorMessage we can remove it.

I would think the signInForm would inherit this error message

src/app/thankYou/accountBenefits/accountBenefits.component.js

src/common/components/signInForm/signInButton/signInButton.component.js

dr-bizz · 2024-02-28T21:38:03Z

src/common/components/signUpActivationModal/signUpActivationModal.component.js

+  }
+
+  $onInit () {
+    if (includes([Roles.identified, Roles.registered], this.sessionService.getRole())) {


Suggested change

if (includes([Roles.identified, Roles.registered], this.sessionService.getRole())) {

if (this.sessionService.getRole() !== 'PUBLIC')) {

This is probably fine since we aren't planning to add more roles anytime soon I think.

dr-bizz · 2024-02-29T18:15:13Z

src/common/components/signUpActivationModal/signUpActivationModal.component.js

+          if (this.unverifiedAccount.email) {
+            this.initialLoading = false
+          }


Test if we need this in If statement or if we should wrap the whole this.sessionService.checkCreateAccountStatus in the email If statement.

dr-bizz · 2024-02-29T18:35:41Z

src/common/components/signUpModal/signUpModal.component.js

+          if (!this.donorDetails.name['given-name'] && angular.isDefined(this.sessionService.session.first_name)) {
+            this.donorDetails.name['given-name'] = this.sessionService.session.first_name
+          }
+          if (!this.donorDetails.name['family-name'] && angular.isDefined(this.sessionService.session.last_name)) {
+            this.donorDetails.name['family-name'] = this.sessionService.session.last_name
+          }
+          if (angular.isUndefined(this.donorDetails.email) && angular.isDefined(this.sessionService.session.email)) {
+            this.donorDetails.email = this.sessionService.session.email


We should do all these the same. angular.isUndefined or !this.donorDetails...

wrandall22 · 2024-02-29T18:37:03Z

src/app/designationEditor/designationEditor.spec.js

-      it('does not broadcast an event if not registered/new', () => {
-        $ctrl.sessionEnforcerService.mock.calls[0][1]['change'](Roles.registered, 'COMPLETED')
-        expect($ctrl.$rootScope.$broadcast).not.toHaveBeenCalled()
+      it('should call onHandleOktaRedirect', () => {


Is this not just a duplicate of the one above now (it('should call onHandleOktaRedirect onInit')?

wrandall22 · 2024-02-29T18:49:03Z

src/common/services/session/sessionHandleOktaRedirect.service.spec.js

+
+      expect($log.error).toHaveBeenCalledWith('Failed to redirect from Okta', 'ERROR_MESSAGE')
+      sessionHandleOktaRedirectService.errorMessageSubject.subscribe((errorMessage) => {
+        expect(errorMessage).toEqual('generic')


Does this actually get called?

wrandall22 · 2024-02-29T18:50:21Z

src/common/services/session/sessionHandleOktaRedirect.service.spec.js

+      expect(sessionService.updateCurrentProfile).toHaveBeenCalled()
+      expect($rootScope.$broadcast).toHaveBeenCalledWith(LoginOktaOnlyEvent, 'register-account')
+    })
+  });


Remove semicolon

src/common/services/api/cart.service.spec.js

src/app/signIn/signIn.spec.js

wrandall22 · 2024-03-01T14:58:07Z

src/common/components/signUpActivationModal/signUpActivationModal.component.spec.js

@@ -189,7 +189,7 @@ describe('signUpActivationModal', function () {
          expect ($ctrl.unverifiedAccount).toEqual({
            ...createAccountDataCookieData,
            ...checkCreateAccountStatusData,
-            status: 'Awaiting Admin approval'
+            status: 'Sending Activation Email'


Here is a case where it would have been helpful to have a constant that we reference instead of hard-coding the language in multiple places.

wrandall22 · 2024-03-01T15:05:26Z

src/app/designationEditor/designationEditor.component.js

+    this.sessionHandleOktaRedirectService.errorMessageSubject.subscribe((errorMessage) => {
+      this.errorMessage = errorMessage
+    })


I'm not seeing this in the master branch. If it fails, it redirects to the home page. However, I do see $ctrl.errorMessage being used on the sign in form. Would that show this error message?

wrandall22 · 2024-03-01T15:12:36Z

src/app/profile/payment-methods/payment-methods.component.js

+    this.sessionHandleOktaRedirectService.errorMessageSubject.subscribe((errorMessage) => {
+      this.errorMessage = errorMessage
+    })


I would think the signInForm would inherit this error message

src/common/components/signInForm/signInButton/signInButton.component.js

wrandall22 · 2024-03-01T15:19:52Z

src/common/components/signUpActivationModal/signUpActivationModal.component.js

+  }
+
+  $onInit () {
+    if (includes([Roles.identified, Roles.registered], this.sessionService.getRole())) {


This is probably fine since we aren't planning to add more roles anytime soon I think.

wrandall22 · 2024-03-01T15:24:19Z

I noticed the last commit didn't make it to stage, maybe due to being out of sync

Note: Do not use the authClient to logout, just have cortex gateway delete the cookies. This matches the current site experience, and also keeps us from having to add a separate logout uri for every branded checkout page.

wrandall22

What are your thoughts about the register-account vs sign-up language differences throughout the code?

src/app/signOut/signOut.component.js

wrandall22 · 2024-03-06T15:36:36Z

src/app/signOut/signOut.tpl.html

+    <div>
+      <loading>
+        <p><translate>Successfully logged out.</translate></p>
+        <p><translate>Redirecting to your previous page...</translate></p>


This should be dynamic language based on the logic in the controller.

src/app/signOut/signOut.spec.js

src/app/main/main.component.js

src/app/thankYou/accountBenefits/accountBenefits.component.spec.js

src/common/components/accountBenefitsModal/accountBenefitsModal.component.js

src/common/components/userMatchModal/userMatchModal.component.js

[EP-2362] Save contact info after sign up

…tal.

[EP-2362] Remove session modal's signup modal

…ger needed the changes that we made.

…d redirect them back to their page prior to login. I've also updated a variable in sessionService and ensure that this page works, which is why a few other files have changed.

src/common/components/registerAccountModal/registerAccountModal.component.js

[EP-2362] Improvements to register account modal

wrandall22 reviewed Dec 11, 2023

View reviewed changes

wrandall22 reviewed Dec 18, 2023

View reviewed changes

src/common/components/signInForm/signInForm.tpl.html Outdated Show resolved Hide resolved

wrandall22 reviewed Dec 18, 2023

View reviewed changes

src/common/components/signUpActivationModal/signUpActivationModal.tpl.html Outdated Show resolved Hide resolved

dr-bizz added the On Staging Will be merged to the staging branch by Github Actions label Dec 19, 2023

wrandall22 changed the title ~~2362 okta~~ EP-2362 - Okta Dec 20, 2023

wrandall22 reviewed Jan 10, 2024

View reviewed changes

src/common/services/session/session.service.js Outdated Show resolved Hide resolved

wrandall22 requested changes Jan 19, 2024

View reviewed changes

wrandall22 reviewed Jan 19, 2024

View reviewed changes

src/common/components/contactInfo/contactInfo.tpl.html Outdated Show resolved Hide resolved

wrandall22 reviewed Jan 19, 2024

View reviewed changes

src/common/components/registerAccountModal/registerAccountModal.tpl.html Outdated Show resolved Hide resolved

wrandall22 requested changes Jan 19, 2024

View reviewed changes

wrandall22 requested changes Jan 22, 2024

View reviewed changes

wrandall22 reviewed Jan 26, 2024

View reviewed changes

src/common/services/session/session.service.js Outdated Show resolved Hide resolved

wrandall22 reviewed Feb 19, 2024

View reviewed changes

src/app/thankYou/accountBenefits/accountBenefits.component.spec.js Outdated Show resolved Hide resolved

dr-bizz commented Mar 1, 2024

View reviewed changes

wrandall22 requested changes Mar 1, 2024

View reviewed changes

dr-bizz force-pushed the 2362-okta branch from c3484bf to b026ba5 Compare March 6, 2024 15:43

wrandall22 added 4 commits March 6, 2024 14:34

Add Okta login on sign-in page

a88bdc4

Use Okta sign out.

f587856

Note: Do not use the authClient to logout, just have cortex gateway delete the cookies. This matches the current site experience, and also keeps us from having to add a separate logout uri for every branded checkout page.

Use Okta url for downgrade

fc6ecd2

Use Okta forgot password feature

4eb1ee8

wrandall22 requested changes Mar 6, 2024

View reviewed changes

dr-bizz added 4 commits March 6, 2024 16:00

sessionEnforcerService on pages that require login

f640594

Built new registerAccount Modal and page with sign-in & userMatch steps

5a1d441

Scss and small amends

157e06e

SignOut functions and updates

f33851f

canac and others added 23 commits January 14, 2025 13:59

Add checkDonorDetails tests

2097ab2

Merge pull request #1130 from CruGlobal/2362-okta-save-contact-info

4475d67

[EP-2362] Save contact info after sign up

Persist errors on the sign up form when switches between steps.

0884cff

Merge branch '2362-okta-daniel' into 2362-okta

c2408c4

Clean up the signUpModal

53ffa4d

Format and lint

08052bc

Fix indentation

b6ac1fc

Make onSignUpWithOkta optional

f19d43e

Removing cart total tests from signUp as we no longer use the cart to…

f304900

…tal.

Merge branch '2362-okta-daniel' into 2362-okta

b96d6b3

Remove unused createAccount method

5ea7b6d

Merge pull request #1134 from CruGlobal/2362-okta-combine-signup-modals

430b985

[EP-2362] Remove session modal's signup modal

Unnest describe blocks

7d107b3

Remove extraneous </div> tag

15efa61

Adjust indentation

2409e61

Make title capitalization consistent

dd71485

Making a registerAccount Modal that is nonDismissible.

cee7258

I have reverted signIn to it's prior state. This is because we no lon…

b215dc5

…ger needed the changes that we made.

Adding page to handle the redirect from okta and authenticate user an…

d1ef17e

…d redirect them back to their page prior to login. I've also updated a variable in sessionService and ensure that this page works, which is why a few other files have changed.

Merge branch '2362-okta-daniel' into 2362-okta

e2ecefb

Skip contact info modal when registration-state is matched

84c21fe

Remove redundant call to checkDonorDetails

feceba4

Fix modal title while loading donor details

db19e84

canac reviewed Jan 16, 2025

View reviewed changes

src/common/components/registerAccountModal/registerAccountModal.component.js Outdated Show resolved Hide resolved

canac and others added 6 commits January 16, 2025 16:34

Merge pull request #1135 from CruGlobal/2362-okta-register-account-modal

f49f428

[EP-2362] Improvements to register account modal

Cleaning up files

e8496af

Adding tests to new Okta Auth Callback page

58694ba

Upgrading @okta/okta-signin-widget

e43869b

Adding signUpModal tests

416a2aa

Merge branch '2362-okta-daniel' into 2362-okta

5e97251

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EP-2362 - Okta #1080

EP-2362 - Okta #1080

dr-bizz commented Dec 11, 2023 •

edited by wrandall22

Loading

wrandall22 commented Dec 11, 2023

wrandall22 Dec 11, 2023

dr-bizz Dec 12, 2023

wrandall22 Dec 12, 2023

wrandall22 commented Dec 20, 2023

wrandall22 commented Jan 11, 2024 •

edited

Loading

wrandall22 commented Jan 12, 2024 •

edited

Loading

wrandall22 left a comment

wrandall22 left a comment

wrandall22 Jan 19, 2024

wrandall22 left a comment

dr-bizz left a comment

dr-bizz Feb 28, 2024

wrandall22 Mar 1, 2024

dr-bizz Feb 28, 2024

wrandall22 Mar 1, 2024

dr-bizz Feb 28, 2024

wrandall22 Mar 1, 2024

dr-bizz Feb 29, 2024

dr-bizz Feb 29, 2024

wrandall22 Feb 29, 2024

wrandall22 Feb 29, 2024

wrandall22 Feb 29, 2024

wrandall22 Mar 1, 2024

wrandall22 Mar 1, 2024

wrandall22 Mar 1, 2024

wrandall22 Mar 1, 2024

wrandall22 commented Mar 1, 2024

wrandall22 left a comment

wrandall22 Mar 6, 2024

	if (includes([Roles.identified, Roles.registered], this.sessionService.getRole())) {
	if (this.sessionService.getRole() !== 'PUBLIC')) {

EP-2362 - Okta #1080

Are you sure you want to change the base?

EP-2362 - Okta #1080

Conversation

dr-bizz commented Dec 11, 2023 • edited by wrandall22 Loading

Description

Sign in

Register

wrandall22 commented Dec 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrandall22 commented Dec 20, 2023

wrandall22 commented Jan 11, 2024 • edited Loading

wrandall22 commented Jan 12, 2024 • edited Loading

wrandall22 left a comment

Choose a reason for hiding this comment

wrandall22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrandall22 left a comment

Choose a reason for hiding this comment

dr-bizz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrandall22 commented Mar 1, 2024

wrandall22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dr-bizz commented Dec 11, 2023 •

edited by wrandall22

Loading

wrandall22 commented Jan 11, 2024 •

edited

Loading

wrandall22 commented Jan 12, 2024 •

edited

Loading