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

Use authentication code grant instead of password grant. #3

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

ray-lee
Copy link
Member

@ray-lee ray-lee commented Sep 19, 2023

What does this do?

This changes the way that tokens are retrieved from the services layer when logging in. The OAuth authorization code grant is now used, instead of the password grant. In order to support this, the session configuration now accepts the new properties:

  • authCode
  • codeVerifier
  • redirectUri

The username and password properties, used to support the password grant, no longer have any effect.

This library is only responsible for obtaining a token, given an authorization code. Obtaining the authorization code should be done outside of this library.

Why are we doing this? (with JIRA link)

CSpace 8.0 upgrades Spring Security to 5.3, and uses Spring Authorization Server instead of the Spring OAuth2 plugin. These upgrades drop support for the OAuth password grant (which has been removed from the latest versions of OAuth). The recommended approach for web apps is to use the authorization code grant.

How should this be tested? Do these changes have associated tests?

This can be tested together with CSpace 8.0 and cspace-ui 9.0. Log in should continue to work with this combination.

Some unit tests still need to be written. These will be in a separate PR.

Dependencies for merging? Releasing to production?

None.

Has the application documentation been updated for these changes?

n/a

Did someone actually run this code to verify it works?

@ray-lee ran this against a local CSpace 8.0 server.

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Patch coverage is 76.92% of modified lines.

Files Changed Coverage
src/session.js 62.50%
src/tokenHelpers.js 100.00%

📢 Thoughts on this report? Let us know!.

@ray-lee ray-lee force-pushed the auth-code-login branch 2 times, most recently from 001738e to 6cbd3c3 Compare September 20, 2023 00:53
@ray-lee ray-lee marked this pull request as ready for review September 20, 2023 01:12
@ray-lee ray-lee merged commit 4338088 into collectionspace:master Sep 20, 2023
1 of 3 checks passed
@ray-lee ray-lee deleted the auth-code-login branch September 20, 2023 01:33
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.

None yet

1 participant