flatcar / nebraska

Update monitor & manager for applications using the Omaha protocol, optimized for Flatcar Container Linux.
https://www.flatcar.org/docs/latest/nebraska
Apache License 2.0
173 stars 43 forks source link

Having to login multiple time with OIDC Authentication #661

Open fbouliane opened 1 year ago

fbouliane commented 1 year ago

Description

When using the OIDC authentication, we discovered there seems to be a problem logging in, causing you to have to try and login multiple time before being able to use the application.

Impact

Having to login multiple time is pretty annoying for the users.

Environment and steps to reproduce

  1. Using nebraska, deployed with OIDC enabled
  2. Load the nebraska UI, you get redirected to the authentication provider
  3. You authenticate on the authentication provider, and get redirected to the nebraska UI.
  4. From the nebraska UI, we see a 401 coming from the /apps endpoint, and you get automatically redirected again to the authentication provider.
  5. Steps 3-4 can go on for 1-2 time before you are logged-in for real (works about 1 time out of 3).
  6. You are successfully logged in in the nebraska UI.

This was reproduced using both the image ghcr.io/kinvolk/nebraska:2.8.6 and the code from the "main" branch.

Expected behavior

I expect to be logged in for real at first try.

Additional information

By debugging the javascript code, I discovered the race condition comes from 2 places.

A. auth.ts

the auth.ts code is responsible from loading the url from the application url's into the localStorage, to make it available to other components. This component is bound using React.useEffect, which occurs at some point after the application is loaded. https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/utils/auth.ts#L74 https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/utils/auth.ts#L81-L88 https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/utils/auth.ts#L10

B. ApplicationsStore.ts

When loading the application, one of the first call is does is the getApplications() call, which is call in the constructor of applications.ts. https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/stores/ApplicationsStore.ts#L18C23-L18C23 https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/stores/ApplicationsStore.ts#L41-L45 https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/api/API.ts#L42 https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/api/API.ts#L300-L304

The problem occurs when the B code is loaded prior the code in A :

The code in https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/api/API.ts#L300-L304 tries to find the token (which is in fact in the URL) and it's not present in the local storage (token=null), therefore emits the request with an empty header, exactly "Authentication: Bearer null" .

The server replies with a HTTP 401, and the javascript code thinks it's disconnected. https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/stores/ApplicationsStore.ts#L52-L54 therefore the code catches-up and redirect to the authentication provider https://github.com/flatcar/nebraska/blob/3c3a467d12dbd6a98c47b6638aad614e6164d1dd/frontend/src/utils/auth.ts#L96-L103

When A is loaded before B, the call on /apps has a proper bearer token and everything goes well.

Other thoughts:

I am not sure how to fix the race condition using this application's architecture (how to wait for the token to be parsed from the URL before calling other endpoints using react event lifecycle). But this patch seems to fix it in my case (Use the token from the url in case it isn't found in the localStorage). https://github.com/fbouliane/nebraska/commit/b4d479f053987e3bbc2ccb8fa9f941059c89070e

things still puzzles me : How long this bug has been there (Code seems to date back from 2 years) Why nobody had this issue before with this ?

fbouliane commented 1 year ago

Was able to patch it locally with the following hack. After this I no longer have the issue, hope it can help someone else. https://github.com/fbouliane/nebraska/commit/b4d479f053987e3bbc2ccb8fa9f941059c89070e Closing the issue for inacitivty.