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

fix(auth): auto attach offline_access only if idp supports #3117

Merged
merged 2 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui/src/features/auth/context/auth-context-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { PropsWithChildren, useMemo } from 'react';

import { authTokenKey, refreshTokenKey } from '@ui/config/auth';

import { extractInfoFromJWT, JWTInfo } from '../utils';
import { extractInfoFromJWT, JWTInfo } from '../jwt-utils';

import { AuthContext, AuthContextType } from './auth-context';

Expand Down
2 changes: 1 addition & 1 deletion ui/src/features/auth/context/auth-context.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import { JWTInfo } from '../utils';
import { JWTInfo } from '../jwt-utils';

export interface AuthContextType {
isLoggedIn: boolean;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { ClientAuth } from 'oauth4webapi';

// https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
export type JWTInfo = {
sub: string;
Expand Down Expand Up @@ -40,9 +38,3 @@ export const getUserEmail = (user?: JWTInfo | null) => {

return meta;
};

export const oidcClientAuth: ClientAuth = () => {
// equivalent function for token_endpoint_auth_method: 'none'
};

export const shouldAllowIdpHttpRequest = () => __UI_VERSION__ === 'development';
15 changes: 6 additions & 9 deletions ui/src/features/auth/oidc-login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import { useLocation } from 'react-router-dom';
import { OIDCConfig } from '@ui/gen/service/v1alpha1/service_pb';

import { useAuthContext } from './context/use-auth-context';
import { oidcClientAuth, shouldAllowIdpHttpRequest as shouldAllowHttpRequest } from './utils';
import {
getOIDCScopes,
oidcClientAuth,
shouldAllowIdpHttpRequest as shouldAllowHttpRequest
} from './oidc-utils';

const codeVerifierKey = 'PKCE_code_verifier';

Expand Down Expand Up @@ -92,14 +96,7 @@ export const OIDCLogin = ({ oidcConfig }: Props) => {
url.searchParams.set('code_challenge_method', 'S256');
url.searchParams.set('redirect_uri', redirectURI);
url.searchParams.set('response_type', 'code');
url.searchParams.set(
'scope',
[
...oidcConfig.scopes,
// Add offline_access scope if it does not exist
...(oidcConfig.scopes.includes('offline_access') ? [] : ['offline_access'])
].join(' ')
);
url.searchParams.set('scope', getOIDCScopes(oidcConfig, as).join(' '));

window.location.replace(url.toString());
};
Expand Down
20 changes: 20 additions & 0 deletions ui/src/features/auth/oidc-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { AuthorizationServer, ClientAuth } from 'oauth4webapi';

import { OIDCConfig } from '@ui/gen/service/v1alpha1/service_pb';

export const oidcClientAuth: ClientAuth = () => {
// equivalent function for token_endpoint_auth_method: 'none'
};

export const shouldAllowIdpHttpRequest = () => __UI_VERSION__ === 'development';

export const getOIDCScopes = (userOIDCConfig: OIDCConfig, idp: AuthorizationServer) => {
const scopes = [...userOIDCConfig.scopes];

// add offline_access scope automatically only if it is supported by IDP
if (!scopes.includes('offline_access') && idp.scopes_supported?.includes('offline_access')) {
scopes.push('offline_access');
}

return scopes;
};
2 changes: 1 addition & 1 deletion ui/src/features/auth/token-renew.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getPublicConfig } from '@ui/gen/service/v1alpha1/service-KargoService_c
import { LoadingState } from '../common';

import { useAuthContext } from './context/use-auth-context';
import { oidcClientAuth, shouldAllowIdpHttpRequest as shouldAllowHttpRequest } from './utils';
import { oidcClientAuth, shouldAllowIdpHttpRequest as shouldAllowHttpRequest } from './oidc-utils';

export const TokenRenew = () => {
const navigate = useNavigate();
Expand Down
2 changes: 1 addition & 1 deletion ui/src/features/common/layout/main-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Outlet } from 'react-router-dom';

import { paths } from '@ui/config/paths';
import { useAuthContext } from '@ui/features/auth/context/use-auth-context';
import { isJWTDirty } from '@ui/features/auth/utils';
import { isJWTDirty } from '@ui/features/auth/jwt-utils';
import { KargoLogo } from '@ui/features/common/logo/logo';

import * as styles from './main-layout.module.less';
Expand Down
2 changes: 1 addition & 1 deletion ui/src/pages/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Navigate } from 'react-router-dom';
import { redirectToQueryParam } from '@ui/config/auth';
import { paths } from '@ui/config/paths';
import { useAuthContext } from '@ui/features/auth/context/use-auth-context';
import { isAdmin, isJWTDirty } from '@ui/features/auth/utils';
import { isAdmin, isJWTDirty } from '@ui/features/auth/jwt-utils';
import { PageTitle } from '@ui/features/common';

export const User = () => {
Expand Down
Loading