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

SDKS-3045 refactor next dont revoke when sso null #267

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

george-bafaloukas-forgerock
Copy link
Contributor

JIRA Ticket

SDKS-3045

Please, link jira ticket here.

Description

When doing Centralized login, there is no SSO Token. Doing a FRSession.authenticate with policy causes a token revokation, breaking the Transactional Authorization flows when using the OIDC ID Token instead of an SSO Token (cause the app doesn't have it)

Definition of Done Checklist:

  • Acceptance criteria is met.
  • All tasks listed in the user story have been completed.
  • Coded to standards.
  • Ensure backward compatibility.
  • API reference docs is updated.
  • Unit tests are written.
  • Integration tests are written.
  • e2e tests are written.
  • Functional spec is written/updated.
  • Example code snippets have been added.
  • Change log updated.
  • Documentation story is created and tracked.
  • Tech debts and remaining tasks are tracked in separated ticket(s).

…s to do the Node.next calls.

Added tests that cover policy auths for both Central and Embedded use cases
Fixed an issue that if Central login happens the Policy Authentications with OAuth2,0 tokens, revoke the OAuth2.0 tokens
Copy link
Contributor

@vahancouver vahancouver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment and a question.

Need to review again after the demo.


func next(completion:@escaping NodeCompletion<FRUser>) {
FRLog.v("Called")
if let currentUser = FRUser.currentUser {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In AuthService before the change, this line was:
if let currentUser = FRUser.currentUser, currentUser.token != nil {
Are we not losing anything by not checking for currentUser.token against nil?

// In this case we are running a transactional authorization flow, the new SSO Token is the same as the originally created one. When running Centralised login, this lived in the browser cookie storage and is unaccesssible from the app
completion(token, nil, nil)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand the change, lines 158-162 are the REAL CHANGE to fix the bug we were having right? The rest is just refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants