🐛 Bug Report: Auth redirect flow immediately requests the /refresh endpoint, and will not allow login if a refresh token is not returned from OAuth2 provider.

mcintoac-aws commented 1 week ago

📜 Description

With the enableExperimentalRedirectFlow option which was first introduced in this PR, after login, a request is immediately made to the /refresh endpoint. This requires the existence of the oauth2-refresh-token session cookie, which is set from the result.session object. If a refresh token is therefore not included in the response from the provider, this cookie is not set, and the requests throws an error which prevents login.

👍 Expected behavior

On login, a request is not made to the /refresh endpoint, and therefore a refresh token is not required to allow initial login. Whether your identity provider doesn't support refresh flow, or doesn't have it enabled, login should still be allowed within the expiration of the original access token.

👎 Actual Behavior with Screenshots

Note that the request headers do not include the proper cookie.

👟 Reproduction steps

Set the enableExperimentalRedirectFlow: true option in your config file, then attempt to login to an identity provider which does not return a refresh token in it's /token response.

📃 Provide the context for the Bug.

This was first introduced in this PR.

A comment chain on the CR between the implementer and @Rugvip seems to be somewhat related, notably:

"The only downside of that is that if the user doesn't go through with the sign-in then it'll cause a request to the /refresh endpoint for that provider on subsequent loads, but I think that's fine." Seems like it could have something to do with the way the provider ID is handled being set in local storage.

🖥️ Your Environment

Browser: Google Chrome

yarn backstage-cli info
yarn run v1.22.21
$ /Users/mcintoac/Documents/repos/code-browser/Connected-mobility-solution-on-aws/source/modules/backstage/node_modules/.bin/backstage-cli info
OS:   Darwin 23.5.0 - darwin/arm64
node: v18.17.1
yarn: 1.22.21
cli:  0.26.6 (installed)
backstage:  1.26.4

Dependencies:
  @backstage/app-defaults                                          1.5.5
  @backstage/backend-app-api                                       0.7.5
  @backstage/backend-common                                        0.21.7, 0.22.0
  @backstage/backend-defaults                                      0.2.18
  @backstage/backend-dev-utils                                     0.1.4
  @backstage/backend-openapi-utils                                 0.1.11
  @backstage/backend-plugin-api                                    0.6.18
  @backstage/backend-tasks                                         0.5.23
  @backstage/backend-test-utils                                    0.3.8
  @backstage/catalog-client                                        1.6.5
  @backstage/catalog-model                                         1.5.0
  @backstage/cli-common                                            0.1.13
  @backstage/cli-node                                              0.2.5
  @backstage/cli                                                   0.26.6
  @backstage/config-loader                                         1.8.0
  @backstage/config                                                1.2.0
  @backstage/core-app-api                                          1.12.5
  @backstage/core-compat-api                                       0.2.5
  @backstage/core-components                                       0.14.7
  @backstage/core-plugin-api                                       1.9.2
  @backstage/dev-utils                                             1.0.32
  @backstage/errors                                                1.2.4
  @backstage/eslint-plugin                                         0.1.8
  @backstage/frontend-plugin-api                                   0.6.5
  @backstage/integration-aws-node                                  0.1.12
  @backstage/integration-react                                     1.1.27
  @backstage/integration                                           1.11.0
  @backstage/plugin-api-docs                                       0.11.5
  @backstage/plugin-app-backend                                    0.3.67
  @backstage/plugin-app-node                                       0.1.18
  @backstage/plugin-auth-backend-module-atlassian-provider         0.1.10
  @backstage/plugin-auth-backend-module-aws-alb-provider           0.1.10
  @backstage/plugin-auth-backend-module-azure-easyauth-provider    0.1.1
  @backstage/plugin-auth-backend-module-bitbucket-provider         0.1.1
  @backstage/plugin-auth-backend-module-cloudflare-access-provider 0.1.1
  @backstage/plugin-auth-backend-module-gcp-iap-provider           0.2.13
  @backstage/plugin-auth-backend-module-github-provider            0.1.15
  @backstage/plugin-auth-backend-module-gitlab-provider            0.1.15
  @backstage/plugin-auth-backend-module-google-provider            0.1.15
  @backstage/plugin-auth-backend-module-guest-provider             0.1.4
  @backstage/plugin-auth-backend-module-microsoft-provider         0.1.13
  @backstage/plugin-auth-backend-module-oauth2-provider            0.1.15
  @backstage/plugin-auth-backend-module-oauth2-proxy-provider      0.1.11
  @backstage/plugin-auth-backend-module-oidc-provider              0.1.9
  @backstage/plugin-auth-backend-module-okta-provider              0.0.11
  @backstage/plugin-auth-backend                                   0.22.5
  @backstage/plugin-auth-node                                      0.4.13
  @backstage/plugin-auth-react                                     0.1.2
  @backstage/plugin-catalog-backend-module-aws                     0.3.13
  @backstage/plugin-catalog-backend-module-scaffolder-entity-model 0.1.16
  @backstage/plugin-catalog-backend                                1.22.0
  @backstage/plugin-catalog-common                                 1.0.23
  @backstage/plugin-catalog-graph                                  0.4.5
  @backstage/plugin-catalog-import                                 0.10.10
  @backstage/plugin-catalog-node                                   1.12.0
  @backstage/plugin-catalog-react                                  1.12.0
  @backstage/plugin-catalog                                        1.20.0
  @backstage/plugin-events-backend                                 0.3.5
  @backstage/plugin-events-node                                    0.3.4
  @backstage/plugin-home-react                                     0.1.13
  @backstage/plugin-home                                           0.7.4
  @backstage/plugin-kubernetes-common                              0.7.6
  @backstage/plugin-org                                            0.6.25
  @backstage/plugin-permission-common                              0.7.13
  @backstage/plugin-permission-node                                0.7.29
  @backstage/plugin-permission-react                               0.4.22
  @backstage/plugin-proxy-backend                                  0.4.16
  @backstage/plugin-scaffolder-backend-module-azure                0.1.10
  @backstage/plugin-scaffolder-backend-module-bitbucket-cloud      0.1.8
  @backstage/plugin-scaffolder-backend-module-bitbucket-server     0.1.8
  @backstage/plugin-scaffolder-backend-module-bitbucket            0.2.8
  @backstage/plugin-scaffolder-backend-module-gerrit               0.1.10
  @backstage/plugin-scaffolder-backend-module-gitea                0.1.8
  @backstage/plugin-scaffolder-backend-module-github               0.2.8
  @backstage/plugin-scaffolder-backend-module-gitlab               0.4.0
  @backstage/plugin-scaffolder-backend                             1.22.7
  @backstage/plugin-scaffolder-common                              1.5.2
  @backstage/plugin-scaffolder-node-test-utils                     0.1.4
  @backstage/plugin-scaffolder-node                                0.4.4
  @backstage/plugin-scaffolder-react                               1.8.6
  @backstage/plugin-scaffolder                                     1.20.1
  @backstage/plugin-search-backend-module-catalog                  0.1.24
  @backstage/plugin-search-backend-module-pg                       0.5.27
  @backstage/plugin-search-backend-module-techdocs                 0.1.23
  @backstage/plugin-search-backend-node                            1.2.23
  @backstage/plugin-search-backend                                 1.5.9
  @backstage/plugin-search-common                                  1.2.11
  @backstage/plugin-search-react                                   1.7.11
  @backstage/plugin-search                                         1.4.11
  @backstage/plugin-techdocs-backend                               1.10.5
  @backstage/plugin-techdocs-module-addons-contrib                 1.1.10
  @backstage/plugin-techdocs-node                                  1.12.4
  @backstage/plugin-techdocs-react                                 1.2.4
  @backstage/plugin-techdocs                                       1.10.5
  @backstage/plugin-user-settings                                  0.8.6
  @backstage/release-manifests                                     0.0.11
  @backstage/repo-tools                                            0.9.0
  @backstage/test-utils                                            1.5.5
  @backstage/theme                                                 0.5.5
  @backstage/types                                                 1.1.1
  @backstage/version-bridge                                        1.0.8
✨  Done in 0.75s.

👀 Have you spent some time to check if this bug has been raised before?

[X] I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

[X] I have read the Code of Conduct

Are you willing to submit PR?

Yes I am willing to submit a PR!

freben commented 5 days ago

Am I understanding it right that even if the immediate refresh wasn't there, users would have a suddenly expired session after a while, and a broken Backstage experience? Just out of curiosity, how long is that expiry? Is this an intermediate state of things or is there some path forward where you can achieve refreshable sessions?

Either way, sure, if there is a way of avoiding this safely (without breaking other assumptions), then that sounds like a useful fix.

mcintoac-aws commented 5 days ago

@freben

I don't think that understanding is entirely correct. I don't believe the refresh flow is broken, so without the immediate refresh, an expired session should ideally refresh correctly.

The issue I am referring to is that the initial login prompts a refresh immediately. So for a provider that does not provide a valid refresh token, the expectation is that login would be allowed, and then when the access token expires (refresh is now necessary), the refresh would fail and the user would be required to log back in. Instead, it is impossible to login at all without a refresh token, which should not be the expected behavior.

I am totally willing and able to submit a fix for this, as it is currently a bit of a blocker for us on our use case, but I am struggling to understand where this immediate refresh occurs.

backstage / backstage