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

Okta | Remove "fallback" option for apps #2451

Merged
merged 1 commit into from
Oct 23, 2023
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
54 changes: 0 additions & 54 deletions docs/okta/native-apps-integration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,11 @@ To setup a native app, we will need to register the application as an client wit
- This is used by the Okta SDK to redirect back to after calling the logout method in the SDK.
- Similar to above we suggest a custom URL scheme for this URI, which we can identify (`your-app:/logout/callback`).
- e.g. `com.theguardian.app.oauth:/logout/callback`
3. _Deprecated_ A redirect URI for handling generic redirects from identity to app as a fallback
- Due to the possibility that Okta may change how things work with their login page, we need a fallback to allow users to sign in should the interception fail
- This endpoint will ONLY be called if the interception fails
- See [fallback](#fallback) section below for more details about this
- We suggest a custom URL scheme for this URI, which we can identify (`your-app://identity/fallback`).
- e.g. `com.theguardian.app://identity/fallback`

No 1. will be handled by the Okta SDK.

No 2. may have to be handled by the application rather than the Okta SDK to handle post logout clean up.

No 3. will have to be handled by the native app itself by registering the custom url scheme.

We will need a set for the PROD, CODE, and possibly DEV environments.

Once the app is set up within Okta and this project. The Identity team will give you the following information to configure the Okta SDK:
Expand Down Expand Up @@ -272,49 +264,3 @@ NativeLayer ->> Okta: SDK uses auth_code to call OAuth /token to get OAuth token
Okta ->> NativeLayer: Return tokens to SDK
note over NativeLayer: the SDK manages the tokens, which can now be used to<br/>authenticate requests, and for checking the user's<br/>session
```

### Fallback (Deprecated)

### This fallback option is no longer required as Okta have changed how they support their sign in page, but is kept here for reference

While we are confident that the above approach is robust enough to work, we will also include a fallback option in case something does go wrong with the above approach.

Specifically a fallback option will be used if we're unable to intercept the `fromURI` parameter from Okta.

To handle this fallback option, a [private-use URI scheme](https://datatracker.ietf.org/doc/html/rfc8252#section-7.1) (referred to as "custom URL scheme") will have to be registered with the app. We suggest a custom URL scheme for this URI, which we can identify (`your-app://identity/fallback`) e.g. `com.theguardian.app://identity/fallback`. This should also be added as a valid redirect URI within the Okta configuration so that we can dynamically redirect to this custom URL scheme (by reading the URL from the Okta API, instead of hardcoding it) from within Gateway.

If after sign in or creating a password, we're unable to redirect to the `fromURI` parameter, we will redirect to the custom URL scheme fallback endpoint once the session has been set in the browser.

When listening on this endpoint in the app, it should just call the `signin`/`signInWithBrowser` method again, with the `prompt=none` parameter. This will still trigger the authorization code flow, and an access token will be returned to the browser, however on iOS the user will see an additional cookie popup, and on android the user will experience a browser flash.

```mermaid
sequenceDiagram
autonumber

participant NativeLayer
participant InAppBrowserTab
participant Gateway
participant Okta

NativeLayer->>InAppBrowserTab: Call `signin`/`signInWithBrowser`<br/>to perform<br />Authorization Code Flow with PKCE
note over InAppBrowserTab: SDK will launch another in-app browser<br/>tab to handle the session check,<br/> and redirect back to the app with auth code
InAppBrowserTab->>Okta: call OAuth /authorize
note over Okta: session check
opt no existing session
Okta->>InAppBrowserTab: Return /login/login.html
note over InAppBrowserTab: Load html, run JS redirect<br>since this is the fallback option, fromURI is not available<br>to /signin or /welcome/:token<br> with clientId param
InAppBrowserTab->>Gateway: Request /signin or /welcome/:token with clientId
Gateway->>InAppBrowserTab: Load /signin or /welcome/:token
note over InAppBrowserTab: User sign in with<br>email+password/social/set password<br>session set in browser<br>fromURI is not available<br>redirect to fallback URI<br>com.theguardian.app://identity/fallback
end
InAppBrowserTab->>NativeLayer: Redirect request com.theguardian.app://identity/fallback
note over NativeLayer: Listen for custom URL scheme<br/>com.theguardian.app://identity/fallback<br>if this is called do the following
NativeLayer->>InAppBrowserTab: Call `signin`/`signInWithBrowser`<br>with `prompt=none` parameter<br/>to perform<br />Authorization Code Flow with PKCE
InAppBrowserTab->>Okta: call OAuth /authorize
note over Okta: session check, session exists
Okta->>InAppBrowserTab: Redirect request to app with the `auth_code` parameter<br/>oauth redirect_uri
InAppBrowserTab->>NativeLayer: Handle redirect request<br/>in-app browser tab to app<br/>oauth redirect_uri?auth_code={code}
NativeLayer->>Okta: SDK uses auth_code to call OAuth /token to get OAuth tokens
Okta->>NativeLayer: Return tokens to SDK
note over NativeLayer: the SDK manages the tokens, which can now be used to<br/>authenticate requests, and for checking the user's<br/>session
```
116 changes: 0 additions & 116 deletions scripts/okta/__tests__/getRedirectUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,122 +383,6 @@ describe('getRedirectUrl', () => {
);
});

it('should return /signin only with clientId if force_fallback is set - identity classic', () => {
expect(
getRedirectUrl(
new URLSearchParams('?force_fallback=true&client_id=test123'),
'https://profile.theguardian.com',
'',
{
getRequestContext: () => ({
target: {
clientId: 'test123',
label: 'jobs_site',
},
authentication: {
request: {
max_age: 100,
},
},
}),
getSignInWidgetConfig: () => ({
relayState: '/testFromURI',
}),
completeLogin: () => {},
},
),
).toBe('/signin?appClientId=test123');
});

it('should return /signin only with clientId if force_fallback is set - identity engine', () => {
expect(
getRedirectUrl(
new URLSearchParams('?force_fallback=true&client_id=test123'),
'https://profile.theguardian.com',
'',
{
getRequestContext: () => ({
app: {
value: {
id: 'test123',
label: 'jobs_site',
},
},
authentication: {
request: {
max_age: 100,
},
},
}),
getSignInWidgetConfig: () => ({
relayState: '/testFromURI',
}),
completeLogin: () => {},
},
),
).toBe('/signin?appClientId=test123');
});

it('should return /welcome/:token only with clientId if force_fallback is set - identity classic', () => {
expect(
getRedirectUrl(
new URLSearchParams(
'?force_fallback=true&client_id=test123&activation_token=123',
),
'https://profile.theguardian.com',
'',
{
getRequestContext: () => ({
target: {
clientId: 'test123',
label: 'jobs_site',
},
authentication: {
request: {
max_age: 100,
},
},
}),
getSignInWidgetConfig: () => ({
relayState: '/testFromURI',
}),
completeLogin: () => {},
},
),
).toBe('/welcome/123?appClientId=test123');
});

it('should return /welcome/:token only with clientId if force_fallback is set - identity engine', () => {
expect(
getRedirectUrl(
new URLSearchParams(
'?force_fallback=true&client_id=test123&activation_token=123',
),
'https://profile.theguardian.com',
'',
{
getRequestContext: () => ({
app: {
value: {
id: 'test123',
label: 'jobs_site',
},
},
authentication: {
request: {
max_age: 100,
},
},
}),
getSignInWidgetConfig: () => ({
relayState: '/testFromURI',
}),
completeLogin: () => {},
},
),
).toBe('/welcome/123?appClientId=test123');
});

it('should get fromURI from location and queryparams if path starts with /oauth2/ and fromURI is not in the okta config - identity classic', () => {
expect(
getRedirectUrl(
Expand Down
7 changes: 2 additions & 5 deletions scripts/okta/lib/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,6 @@ export const getRedirectUrl = (
// set up params class to hold the parameters we'll be passing to our own login page
const params = new URLSearchParams();

// force fallback flag, used to test fallback behaviour
const forceFallback = searchParams.get('force_fallback');

// Variable holders for the Okta params we want to pass to our own login page
// This is the URI to redirect to after the user has logged in and has a session set to complete the Authorization Code Flow from the SDK.
let fromURI: string | undefined;
Expand All @@ -207,7 +204,7 @@ export const getRedirectUrl = (
let maxAge: number | undefined;

// attempt to get the parameters we need from the Okta hosted login page OktaUtil object
if (oktaUtil && !forceFallback) {
if (oktaUtil) {
// try getting fromURI from OktaUtil signInWidgetConfig (property is called called relayState)
const signInWidgetConfig = oktaUtil.getSignInWidgetConfig();
fromURI = getRelayState(signInWidgetConfig);
Expand All @@ -228,7 +225,7 @@ export const getRedirectUrl = (
}

// if we're unable to get clientId from OktaUtil, try to get it from the search params where it will exist
if (!clientId || forceFallback) {
if (!clientId) {
clientId = searchParams.get('client_id') || undefined;
}

Expand Down
2 changes: 1 addition & 1 deletion scripts/okta/okta-login.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 25 additions & 29 deletions src/client/.well-known/apple-app-site-association
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
{
"applinks": {
"details": [
{
"appIDs": [
"U9LTYR56M6.uk.co.guardian.iphone2",
"998P9U5NGJ.uk.co.guardian.iphone2.debug",
"998P9U5NGJ.uk.co.guardian.iphone2.debugqa"
],
"components": [
{
"/": "/welcome/il_*",
"comment": "Matches any URL whose path starts with /welcome/il_<randomString> where <randomString> is a token"
},
{
"/": "/reset-password/il_*",
"comment": "Matches any URL whose path starts with /reset-password/il_<randomString> where <randomString> is a token"
},
{
"/": "/set-password/il_*",
"comment": "Matches any URL whose path starts with /set-password/il_<randomString> where <randomString> is a token"
},
{
"/": "/identity/fallback/*",
"comment": "Matches any URL whose path starts with /identity/fallback/"
}
]
}
]
}
"applinks": {
"details": [
{
"appIDs": [
"U9LTYR56M6.uk.co.guardian.iphone2",
"998P9U5NGJ.uk.co.guardian.iphone2.debug",
"998P9U5NGJ.uk.co.guardian.iphone2.debugqa"
],
"components": [
{
"/": "/welcome/il_*",
"comment": "Matches any URL whose path starts with /welcome/il_<randomString> where <randomString> is a token"
},
{
"/": "/reset-password/il_*",
"comment": "Matches any URL whose path starts with /reset-password/il_<randomString> where <randomString> is a token"
},
{
"/": "/set-password/il_*",
"comment": "Matches any URL whose path starts with /set-password/il_<randomString> where <randomString> is a token"
}
]
}
]
}
}
34 changes: 0 additions & 34 deletions src/server/routes/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { addQueryParamsToPath } from '@/shared/lib/queryParams';
import postSignInController from '@/server/lib/postSignInController';
import { IdTokenClaims, TokenSet } from 'openid-client';
import { updateUser } from '@/server/lib/okta/api/users';
import { getApp } from '@/server/lib/okta/api/apps';
import { setUserFeatureCookies } from '@/server/lib/user-features';
import { consentPages } from './consents';
import {
Expand Down Expand Up @@ -191,39 +190,6 @@ const authenticationHandler = async (
return res.redirect(303, authState.queryParams.fromURI);
}

// fallback option for apps
// if we can't get the fromURI from Okta, we can still use the client id to redirect
// back to the application, where they can use the session cookie to complete the flow
// by calling the signin/signinwithbrowser sdk method with the prompt=none parameter
// firstly check if we have the client id parameter
if (authState.queryParams.appClientId) {
try {
// attempt to find the native app by the client id
const nativeApp = await getApp(authState.queryParams.appClientId);

// Check for fallback link if found
if (nativeApp) {
// check if the fallback link is set
const fallbackUrl = nativeApp.settings.oauthClient.redirect_uris.find(
(url) => url.includes('://identity/fallback'),
);
// if the fallback link is set, redirect to it
if (fallbackUrl) {
return res.redirect(303, fallbackUrl);
}
}
} catch (error) {
// catch if the getApp call fails, we log error, but fall through to prevent runtime errors
logger.error(
`Failed to get app config for app ${authState.queryParams.appClientId}`,
error,
{
request_id: res.locals.requestId,
},
);
}
}

const returnUrl = authState.confirmationPage
? addQueryParamsToPath(authState.confirmationPage, authState.queryParams)
: authState.queryParams.returnUrl;
Expand Down