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

Specification of session in SDK #1391

Closed
6 tasks done
fungc-io opened this issue Jul 26, 2021 · 9 comments
Closed
6 tasks done

Specification of session in SDK #1391

fungc-io opened this issue Jul 26, 2021 · 9 comments

Comments

@fungc-io
Copy link
Member

fungc-io commented Jul 26, 2021

Progress

Server changes

The SDK issues are tracked by

Study

Overview of the SDK configuration

The mobile SDK has 2 options related to token storage and interaction with the system browser. (* indicates the default value)

  1. tokenStorage: TransientTokenStorage | PersistentTokenStorage* | AppGroupPersistentTokenStorage | AccountManagerPersistentTokenStorage
  2. shareSessionWithSystemBrowser: true | false*

The web SDK has 1 option related to session (* indicates the default value)

  1. sessionType: refresh_token* | cookie

TransientTokenStorage

TransientTokenStorage stores the refresh token in memory. In other words, the end-user must authenticate themselves on app launch. This option is intended for applications like banking applications.

PersistentTokenStorage

PersistentTokenStorage stores the refresh token in app-specific storage. The is the default value. This option is intended for general applications.

AppGroupPersistentTokenStorage (iOS specific implementation)

AppGroupPersistentTokenStorage stores the refresh token in the keychain shared by apps in the same app group. We call the apps in the same app group affiliated apps. Affiliated apps share the SAME refresh token. Signing in in one app affects all affiliated apps.

AccountManagerPersistentTokenStorage (Android specific implementation)

AccountManagerPersistentTokenStorage stores the refresh token with the Android Account Manager API. The account data can only be read by apps signed by the same key. We call the apps signed by the same key affiliated apps. Affiliated apps share the SAME refresh token. Signing in in one app affects all affiliated apps.

Since affiliated apps can be opened concurrently, platform specific application lifecycle callback MUST BE set up properly by the developer to correctly reflect the state when switching app. For example, on iOS, the SDK provides applicationDidBecomeActive and sceneDidBecomeActive to sync state when app is brought to foreground.

The developer must also listen for the SDK sessionStateChange event to make sure the application is aware of the latest state.

shareSessionWithSystemBrowser = false

When shareSessionWithSystemBrowser is false, the SDK asks the server to suppress IDP session cookie. The SDK also uses an ephemeral web browser session if available.

shareSessionWithSystemBrowser = true

When shareSessionWithSystemBrowser is true, IDP session cookie is written as usual. The SDK uses a suitable webview so that the cookie is reflected in the system browser. Therefore, signing in an app will have a by-product of signing in on the web. On the other hand, if the end-user signs in on the website first, when they signs in in the app, they will see the continue screen.

Use case examples

Here we list a number of use cases and their corresponding configuration.

Use case 1: An application that requires authentication on every app launch

  • tokenStorage: TransientTokenStorage
  • shareSessionWithSystemBrowser: false*

Typically this type of application advocates biometric authentication. The end-user authenticates with biometric.

Use case 2: Standalone mobile app

  • tokenStorage: PersistentTokenStorage*
  • shareSessionWithSystemBrowser: false*

This is the most common use case, so the default values are designed to work with this use case.

Use case 3: A collection of apps that shares signed in status, e.g. The series of apps developed by Google

  • tokenStorage: AppGroupPersistentTokenStorage | AccountManagerPersistentTokenStorage
  • shareSessionWithSystemBrowser: false*

This is an advanced use case due to the complexity of setting up affiliated apps.

Use case 4: A website and a mobile app

  • tokenStorage: PersistentTokenStorage*
  • shareSessionWithSystemBrowser: true

A typical example is a service available both as a website and a mobile app. When the end-user signs in the mobile app, the website will also be signed in. This is useful when the website and the mobile app implements Universal link. When the end-user receive a universal link, the link will first be opened in the website (signed in). Then the end-user can choose to open in app instead. Having signed in state in the website helps tracking the end-user.

Implementation details of the options

tokenStorage is merely an option controlling how refresh token is stored. It does not affect other things.

shareSessionWithSystemBrowser is summarized as follows:

iOS Android
shareSessionWithSystemBrowser = false ASWebAuthenticationSession, prefersEphemeralWebBrowserSession=true, x_suppress_idp_session_cookie=true CustomTabs, x_suppress_idp_session_cookie=true
shareSessionWithSystemBrowser = true ASWebAuthenticationSession CustomTabs

Related SDK API

The mobile SDK has only one API to log out. It is logout(). It always revokes the refresh token it is using.

Portal changes

Right now the IDP session settings and individual application session settings are NOT co-located. It is proposed to move Session to Applications as a new pane. The title will be changed to "Configure cookie for ". Inside application configuration screen, there are 3 panes. The 1st pane contains client_id, redirect_uris and name. The 2nd pane contains refresh token settings titled as "Mobile / SPA". The 3rd pane contains post_logout_redirect_uris titled as "Cookie"

@carmenlau
Copy link
Contributor

After research, android custom chrome tab doesn't have prefersEphemeralWebBrowserSession like iOS. Using WebView is not recommended either.

WebView is explicitly not supported due to usability and security reasons.

refs: https://github.com/openid/AppAuth-Android/blob/daeb09c/README.md

Apps can initiate an authorization request in the browser, without
the user leaving the app, through the Android Custom Tab feature,
which implements the in-app browser tab pattern. The user's default
browser can be used to handle requests when no browser supports
Custom Tabs.

refs: https://datatracker.ietf.org/doc/html/rfc8252#page-19

After discussion with @louischan-oursky, since we cannot isolate the cookies from the client-side in Android, there is an idea that to isolate the cookies with a prefix provided through the query parameter.

When the user visits an entry point (e.g. authorize endpoint/settings page) with query parameter like cookie_prefix=somekey_. All the links in the pages and all redirections will have the same query parameter. All the requests with this query parameter will set and write cookies with the given prefix, so the cookie will be scoped with the prefix.

The cons of this approach are it adds extra complexity for features that need to handle requests from another party. e.g. SSO callback. When the SSO providers call the authgear callback, we cannot add the cookie_prefix query parameter, so we will need to have extra handling to put this information to SSO state and resume it from the callback. If there are similar cases in the future, we may need extra handling as well.

Still thinking if there is any other simpler approach..

@chpapa
Copy link
Member

chpapa commented Jul 26, 2021

@carmenlau @louischan-oursky What about we just support WebView anyway? Both iOS and Android have the same recommendation at AppAuth which say it is "explicitly not supported", but we know (at least on iOS) it would be needed since some users explicitly want to avoid the popup or even embed it in UI although it is against the best practices.

Plus we don't have to add additional complexity

p.s. On the reason why WebView was not explicitly supported, it is mainly related with 8.12 at RFC 8252; I think in summary, there are 3 reasons against that:

  1. unsafe at use by third party "by definition", it is untrusted since third party app can capture key strokes, bypass user consent, etc
  2. as first-party app, it violates the principle of least privilege, as it increases the attack surface
  3. it encourages users to input credentials in a site they can't confirm it is legitimate (or train them to do so)

For our users, most of the time it is used by the same parties, so 1 is not valid; Problem is 2 and 3...

So I think it is a trade off between users expectation? As long as we provide a simple to switch and viable options, I prefer we don't add additional complexity for now on our applications.

@carmenlau
Copy link
Contributor

If this is the case, maybe keep the decision to support webview. But we may want secure by default, so the default will be as following

  • custom chrome tab for android
  • aswebauthenticationsession + prefersEphemeralWebBrowserSession for ios?

What do you think? @chpapa @louischan-oursky

@louischan-oursky
Copy link
Collaborator

In the product meeting on 2021-08-02, Ben raised a concern that just providing the the low-level options useWebView and prefersEphemeralWebBrowserSession may not be developer friendly enough. Normally the developer would have 2 use cases "Sign in on the Device" and "Sign in within the App" in their mind.

Sign in on the Device

This means the logged in session is global on the device. On iOS this translates to ASWebAuthenticationSession with prefersEphemeralWebBrowserSession=false. On Android this translates to Custom Tabs.

Sign in within the App

This means the logged in session is scoped to the application only. On Android this translates to WebView.

On iOS, it could translate to ASWebAuthenticationSession with prefersEphemeralWebBrowserSession=true, but the logged in session is never remembered. Or it could translate to WKWebView.

Summary

How should we present the API? If we keep our low-level options, the curious developer can understand how the implementation looks like, but they need to read the documentation to achieve their intended use cases.

@fungc-io fungc-io added this to To do in 2021_08_09-08_20 Sprint via automation Aug 6, 2021
@fungc-io
Copy link
Member Author

fungc-io commented Aug 10, 2021

In the mobile app SDK, replace the existing option transientSession: boolean with sessionType: SessionType

SessionType Login behavior Logout behavior iOS Android
transient Do NOT persist refresh token; Do NOT write IDP session cookie Revoke refresh token ASWebAuthenticationSession with prefersEphemeralWebBrowserSession WebView
app (Default value) Persist refresh token; Write IDP session cookie Revoke refresh token WebView WebView
device Persist refresh token; Write IDP session cookie Open webview to logout and then revoke refresh token, see #1467 ASWebAuthenticationSession CustomTabs

Note that the current behavior is equivalent to SessionType.device, and the default session type is now SessionType.app. The developer have to "opt-in" if they want to use SSO on the device. Also note that this design does not support the use case "The user wants to log out from the app but not the device". This use case is considered rare as the majority of the users is using Authgear as the first-party authentication provider.

Since IDP session cookie is now optional, we can no longer assume the existence of the IDP session cookie. This is tracked by

Additionally, the server must let the SDK to control whether the IDP session cookie is written. This is tracked by

The SDK issues are tracked by

@louischan-oursky louischan-oursky changed the title Update SDK to support cookie sharing settings Redesign of the SDK SessionType option Aug 12, 2021
@louischan-oursky louischan-oursky changed the title Redesign of the SDK SessionType option Redesign the SDK SessionType option Aug 12, 2021
@louischan-oursky louischan-oursky moved this from To do to In progress in 2021_08_09-08_20 Sprint Aug 20, 2021
@fungc-io fungc-io added this to To do in 2021_08_23-09_03 Sprint via automation Aug 20, 2021
@louischan-oursky louischan-oursky moved this from To do to In progress in 2021_08_23-09_03 Sprint Aug 24, 2021
@louischan-oursky louischan-oursky changed the title Redesign the SDK SessionType option Specification of session in SDK Aug 24, 2021
@louischan-oursky louischan-oursky moved this from In progress to Review in progress in 2021_08_23-09_03 Sprint Aug 30, 2021
@fungc-io
Copy link
Member Author

fungc-io commented Sep 1, 2021

New suggestion from @louischan-oursky

typescript
import authgear, {
  ICredentialStorage,
  MemoryCredentialStorage,
  AppScopedCredentialStorage,
  AppGroupCredentialStorage,
} from "@authgear/react-native";

// It is possible that the developer can implement a custom storage themselves.
interface ICredentialStorage {
  // implementation of get(), set() and del().
}

function main() {
  authgear.configure({
    credentialStorage: new MemoryCredentialStorage(),
  });
}

@louischan-oursky louischan-oursky moved this from Review in progress to In progress in 2021_08_23-09_03 Sprint Sep 1, 2021
@louischan-oursky
Copy link
Collaborator

Naming of CredentialStorage

Terry says credential usually means username and password.
TokenStorage would be a better name.

Banking application

  • KenChan: TransientCredentialStorage
  • Terry: TransientTokenStorage; anotnym of persistent
  • Rocky: MemoryCredentialStorage
  • Teddy: TransientTokenStorage
  • Edmond: No idea

Application likes Facebook

  • KenChan: PersistentCredentialStorage
  • Terry: PersistentTokenStorage
  • Rocky: PersistentCredentialStorage
  • Teddy: PersistentCredentialStorage
  • Edmond: SingleAppCredentialStorage or AppCredentialStorage

Google apps

  • KenChan: CrossAppCredentialStorage or MultipleAppCredentialStorage
  • Terry: SharedTokenStorage or SynchronizedTokenStorage; the use case means share.
  • Rocky: AppGroupCredentialStorage; Rocky knows very well what App Group is in iOS. Or SharedCredentialStorage if AppGroupCredentialStorage is absent.
  • Teddy: AffiliatedAppsCredentialStorage. Affiliated is better than associated because it fits the use case more.
  • Edmond: AffiliatedAppsCredentialStorage. Reason same as Teddy.

Sharing cookie bi-directionally with system browser

  • KenChan: rememberSessionInDeviceBrowser
  • Terry: createSessionInSystemBrowser or generateSessionInSystemBrowser
  • Rocky: shareSessionWithSystemBrowser
  • Teddy: shareSessionWithSystemBrowser
  • Edmond: shareSessionWithSystemBrowser

All of them do not prefer Cookie because it involves implementation details.
Most of them think AuthgearSession is too verbose and redundant.
All of them agree SystemBrowser is better because only Safari on iOS and Chrome on Android are affected.

Rocky is the only developer who asked me about the directionality.
He questioned if the end-user signed in on the website first,
what would the end-user see if they are signing in in the mobile app.
After he knew it is bi-directional, he thinks "share" is the best term.

Conclusion

I agree with Terry that TokenStorage is better than CredentialStorage as the former is more accurate.
It is no doubt that PersistentTokenStorage is the best choice.
If PersistentTokenStorage is chosen, then its anotnym TransientTokenStorage should be easy to understand.
The app group use case is very controversial, and there is no clear winner.
In my opinion, SharedTokenStorage should be a more common term than AffiliatedAppsTokenStorage.

shareSessionWithSystemBrowser seems to be the best choice we have.

In short, we have

  • TransientTokenStorage
  • PersistentTokenStorage
  • SharedTokenStorage
  • shareSessionWithSystemBrowser

@louischan-oursky louischan-oursky moved this from In progress to Review in progress in 2021_08_23-09_03 Sprint Sep 1, 2021
@fungc-io
Copy link
Member Author

fungc-io commented Sep 1, 2021

I agree that the "credentials" sounds like username+password.
👍 Let's go for the naming in your conclusion.

@louischan-oursky louischan-oursky moved this from Review in progress to In progress in 2021_08_23-09_03 Sprint Sep 2, 2021
@louischan-oursky
Copy link
Collaborator

After listening to Ben's comments, I came up with a new API that uses platform specific terms as well as mentioning the underlying implementation. Hope that will help the developer to understand if they are familiar with the used terms.

Here is the pseudo code illustrating the API

import { Platform } from "react-native";
import authgear, {
  AppGroupPersistentTokenStorage,
  AccountManagerPersistentTokenStorage,
  PersistentTokenStorage,
  TransientTokenStorage,
} from "@authgear/react-native";

// When the developer is writing a banking application, they would write
authgear.configure({
  tokenStorage: new TransientTokenStorage(),
});

// When the developer has a collection of apps that should share logged in session, they would write
authgear.configure({
  // Platform.select is a well-known react-native idiom to write platform-specific code.
  // See https://reactnative.dev/docs/platform-specific-code
  // AppGroupPersistentTokenStorage mentions the iOS term App Group in its name to signify its nature.
  // AccountManagerPersistentTokenStorage mentions the implementation details Android Account Manager API.
  tokenStorage: Platform.select({
    ios: new AppGroupPersistentTokenStorage(...),
    android: new AccountManagerPersistentTokenStorage(...),
  }),
});

// Otherwise PersistentTokenStorage is the default storage if storage is not specified.

@fungc-io fungc-io added this to To do in 2021_09_06-09_17 Sprint via automation Sep 3, 2021
@louischan-oursky louischan-oursky moved this from In progress to Review in progress in 2021_08_23-09_03 Sprint Sep 6, 2021
@fungc-io fungc-io moved this from To do to Review in progress in 2021_09_06-09_17 Sprint Sep 6, 2021
@fungc-io fungc-io closed this as completed Sep 6, 2021
2021_09_06-09_17 Sprint automation moved this from Review in progress to Staging Sep 6, 2021
@fungc-io fungc-io moved this from Staging to Done in 2021_09_06-09_17 Sprint Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

4 participants