Skip to content

Add @atlas/auth package - #209

Open
douglaswinter wants to merge 11 commits into
mainfrom
dw/auth
Open

douglaswinter wants to merge 11 commits into
mainfrom
dw/auth

Conversation

@douglaswinter

Copy link
Copy Markdown
Collaborator

New package is introduced to give our apps a little more awareness and control over auth, while avoiding handling access tokens as Oauth2 Proxy still injects those in the backend.

The base helm chart is modified so that Oauth2 Proxy injects the identity token into response headers, read by @atlas/auth, as well as ensuring api calls (defined as calls to '/api* and /oauth2* return 401 when not authenticated, instead of Oauth2 Proxy redirecting to keycloak itself.

app-shell now uses this package and renders a login button in the app bar. Note that this requires apps that use app-shell to use the context provider given by @atlas/auth.

The new package provides helpers for plain fetch and axios to redirect on 401. This is used in the @atlas/blueapi client, as well as the relay and apollo clients for p99 and i15-1.

1) Inject id token into response headers, so that in JS we can display
   your name.
2) Specify api_routes /oauth2 and /api. This makes calling those without
   a token (i.e. before logging in) returns 401 Unauthorized instead of
   redirecting to Keycloak.
3) skip_auth_routes on everything that isn't /api* or /oauth2*. This
   prevents odd behaviour where / is deemed public but react-router
   redirects to, say, /welcome and then refreshing on /welcome makes
   oauth2 proxy unhappy. We will handle 401's explictly.
A small abstraction giving our apps better control over login/logout,
and visibility of the user. Two implementations are available: oauth2
proxy, and a mock for dev servers. Note that this does not handle access
tokens (oauth2 proxy is still doing its magic).
And fix axios helper export
Simply redirecting back to index if not authenticated.

@NKatti2011 NKatti2011 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes requested


export function Layout(props: RouterProps) {
const { isLoading, isAuthenticated } = useAuth();
if (isLoading) return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably would recommend hooks to be together like so:

Suggested change
if (isLoading) return null;
const { isLoading, isAuthenticated } = useAuth();
const { open, setOpen } = usePersistentDrawerState();
if (isLoading) return null;


const getQueueState = async (): Promise<QueueState> => {
const response = await axios.get<QueueState>(QUEUE_SOCKET + "/queue/state");
const response = await axios.get<QueueState>(QUEUE_URL + "/queue/state");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

getQueueState, patchQueueState etc does not seem to be wrapped with queueClient (for redirect) - should the bare axios be replaced with queueClient here?

isLoading: boolean;
login: (returnTo?: string) => void;
logout: (returnTo?: string) => void;
/** Re-run the auth check, e.g. after the tab regains focus. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nitpicky (related to comment): refresh should run only for when auth has changed rather than if tab regains focused since app will entirely unmount.

<QueryClientProvider client={new QueryClient()}>
<UserAuthProvider>
<AuthContextProvider provider={authProvider}>
<ReduxProvider store={store(config)}>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ReduxProvider and QueryClientProvider likely to rerender via useLoadPvwsConfig. Expected behavior?

const USERINFO_ENDPOINT = "/oauth2/userinfo";
const HEADERS_CHECK_ENDPOINT = "/auth/me";
const LOGIN_ENDPOINT = "/oauth2/start";
const LOGOUT_ENDPOINT = "/oauth2/sign_out";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wanted to check if the user is also redirected to the identity provider's sign out page (as oauth2/sign_out only removed oauth2-proxy's own cookies):

as suggested here :

To sign the user out, redirect them to /oauth2/sign_out. This endpoint only removes oauth2-proxy's own cookies, i.e. the user is still logged in with the authentication provider and may automatically re-login when accessing the application again. You will also need to redirect the user to the authentication provider's sign-out page afterward using the rd query parameter
(The "sign_out_page" should be the end_session_endpoint from the metadata if your OIDC provider supports Session Management and Discovery.)


/**
* Wraps login() so it fires at most once,
* no matter how many requests 401 around the same time

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since the backends do their own auth, a backend could return a 401 even though the user's oauth2-proxy session is still valid. Perhaps before redirecting we could check with oauth2-proxy that the session is actually gone?

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.

2 participants