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

auth-node: Refresh handler not returning persisted scope in response #20364

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

kunickiaj
Copy link
Contributor

@kunickiaj kunickiaj commented Oct 4, 2023

Fix OAuth refresh handler in auth-node prompting for re-login on page reload.

The refresh handler is returning an empty scope if scope was previously saved in a cookie. The session is successfully refreshed but the client receives a response without the scope it requested, prompting a new login.

This should also be backported to 1.18.x

Resolves #20322

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

The refresh handler is returning an empty scope if scope was previously
saved in a cookie. The session is successfully refreshed but the client
receives a response without the scope it requested, prompting a new
login.

Resolves backstage#20322

Signed-off-by: Adam Kunicki <kunickiaj@gmail.com>
@backstage-goalie
Copy link
Contributor

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/plugin-auth-node plugins/auth-node patch v0.3.2-next.1

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Uffizzi Preview deployment-37636 was deleted.

Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Great find and fix! 🎉 🙏

Thank you for digging into this issue, been as bit occupied by CI failures and other ongoing things 😅 ❤️

@Rugvip Rugvip merged commit 39d9268 into backstage:master Oct 4, 2023
24 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.19.0 release, scheduled for Tue, 17 Oct 2023.

Rugvip added a commit that referenced this pull request Oct 4, 2023
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
@Rugvip Rugvip mentioned this pull request Oct 4, 2023
@Rugvip
Copy link
Member

Rugvip commented Oct 4, 2023

Patch release in #20367

Rugvip added a commit that referenced this pull request Oct 4, 2023
@xyzmurali
Copy link

xyzmurali commented May 6, 2024

Hi, We are seeing a similar issue where this issue where logging in to github through 'backstage-plugin-github-pull-requests' widget also logs in to github authenticator provider, But logging in to github authenticator provider does not login to github for 'backstage-plugin-github-pull-requests' widget

This issue can be reproduced with below steps.

  1. Go to page where Github authenticator sign in present

  2. Login to github using Github authenticator

  3. Go to page where 'backstage-plugin-github-pull-requests' widget is present. The github login should happen automatically and results should be displayed. But it asks for github login again on this page with no results displayed in the widget.

  4. Go to page where Github authenticator sign in present

  5. Log out of gihub

  6. Go to page where 'backstage-plugin-github-pull-requests' widget is present.

  7. Login to github from 'backstage-plugin-github-pull-requests' widget.

  8. Go to page where Github authenticator sign in present. It will show already signed in

I see that the fix for "scope: result.session.scope," was made only to one location in file plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts but it is present at two locations. Can this be an issue?

@awanlin
Copy link
Collaborator

awanlin commented May 6, 2024

@xyzmurali it would be best to log this as an issue as it's not going to be followed up easily here 👍

@xyzmurali
Copy link

Thank you, i submitted issue #24652

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.

🐛 Bug Report: auth.getAccessToken doesn't survive page reloads anymore
4 participants