Skip to content

[HIGH] security(android): isolate interactive request state #1139

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.

Sign up for GitHub

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

Open
OskarEichler wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[HIGH] security(android): isolate interactive request state #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/appauth-android-request-lifecycle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-native-app-auth": patch
---

Snapshot token headers, TLS policy, timeout, parameters, client authentication, PKCE verifier and promise per interactive Android flow. Keep refresh and registration independent, reject overlapping browser flows without replacing the first, and settle late token failures on their originating promise.
9 changes: 9 additions & 0 deletions docs/docs/usage/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,12 @@ See specific example [configurations for your provider](/docs/category/providers
- **androidAllowCustomBrowsers** - (`string[]`) (default: undefined) _ANDROID_ override the used browser for authorization. If no value is provided, all browsers are allowed.
- **androidTrustedWebActivity** - (`boolean`) (default: `false`) _ANDROID_ Use [`EXTRA_LAUNCH_AS_TRUSTED_WEB_ACTIVITY`](https://developer.chrome.com/docs/android/trusted-web-activity/) when opening web view.
- **connectionTimeoutSeconds** - (`number`) configure the request timeout interval in seconds. This must be a positive number. The default values are 60 seconds on iOS and 15 seconds on Android.

### Android request isolation

Token-exchange options belong to each call. Refresh and registration may run while authorization is
pending without replacing its token-exchange parameters or timeout.

Only one browser-based authorization or logout can be pending at a time. A second interactive call
rejects with `authentication_in_progress`; finish or cancel the first before retrying. A token exchange
already running after the browser returns keeps its own promise and configuration.
199 changes: 92 additions & 107 deletions packages/react-native-app-auth/android/src/main/java/com/rnappauth/RNAppAuthModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,27 +65,43 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;

public class RNAppAuthModule extends ReactContextBaseJavaModule implements ActivityEventListener {

public static final String CUSTOM_TAB_PACKAGE_NAME = "com.android.chrome";

private final ReactApplicationContext reactContext;
private Promise promise;
private boolean dangerouslyAllowInsecureHttpRequests;
private Boolean skipCodeExchange;
private Boolean usePKCE;
private Boolean useNonce;
private String codeVerifier;
private String clientAuthMethod = "basic";
private final AtomicReference<PendingFlow> pendingFlow = new AtomicReference<>();
private Map<String, String> registrationRequestHeaders = null;
private Map<String, String> authorizationRequestHeaders = null;
private Map<String, String> tokenRequestHeaders = null;
private Map<String, String> additionalParametersMap;
private String clientSecret;
private final ConcurrentHashMap<String, AuthorizationServiceConfiguration> mServiceConfigurations = new ConcurrentHashMap<>();
private boolean isPrefetched = false;

private static final class PendingFlow {
final Promise promise;
final int requestCode;
final AppAuthConfiguration tokenConfiguration;
final Map<String, String> additionalParameters;
final String clientSecret;
final String clientAuthMethod;
final boolean skipCodeExchange;
String codeVerifier;

PendingFlow(Promise promise, int requestCode, AppAuthConfiguration tokenConfiguration,
Map<String, String> additionalParameters, String clientSecret,
String clientAuthMethod, boolean skipCodeExchange) {
this.promise = promise;
this.requestCode = requestCode;
this.tokenConfiguration = tokenConfiguration;
this.additionalParameters = additionalParameters;
this.clientSecret = clientSecret;
this.clientAuthMethod = clientAuthMethod;
this.skipCodeExchange = skipCodeExchange;
}
}

public RNAppAuthModule(ReactApplicationContext reactContext) {
super(reactContext);
this.reactContext = reactContext;
Expand Down Expand Up @@ -253,15 +269,16 @@ public void authorize(
dangerouslyAllowInsecureHttpRequests, androidAllowCustomBrowsers);
final HashMap<String, String> additionalParametersMap = MapUtil.readableMapToHashMap(additionalParameters);

// store args in private fields for later use in onActivityResult handler
this.promise = promise;
this.dangerouslyAllowInsecureHttpRequests = dangerouslyAllowInsecureHttpRequests;
this.additionalParametersMap = additionalParametersMap;
this.clientSecret = clientSecret;
this.clientAuthMethod = clientAuthMethod;
this.skipCodeExchange = skipCodeExchange;
this.useNonce = useNonce;
this.usePKCE = usePKCE;
final AppAuthConfiguration tokenConfiguration = createAppAuthConfiguration(
createConnectionBuilder(dangerouslyAllowInsecureHttpRequests,
this.tokenRequestHeaders, connectionTimeoutMillis),
dangerouslyAllowInsecureHttpRequests, null);
final PendingFlow flow = new PendingFlow(promise, 52, tokenConfiguration,
additionalParametersMap, clientSecret, clientAuthMethod, Boolean.TRUE.equals(skipCodeExchange));
if (!pendingFlow.compareAndSet(null, flow)) {
promise.reject("authentication_in_progress", "Another authorization or logout is already in progress");
return;
}

// when serviceConfiguration is provided, we don't need to hit up the OpenID
// well-known id endpoint
Expand All @@ -280,10 +297,13 @@ public void authorize(
usePKCE,
additionalParametersMap,
androidTrustedWebActivity,
androidPrefersEphemeralSession);
androidPrefersEphemeralSession,
flow);
} catch (ActivityNotFoundException e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("browser_not_found", e.getMessage());
} catch (Exception e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("authentication_failed", e.getMessage());
}
} else {
Expand All @@ -295,6 +315,7 @@ public void onFetchConfigurationCompleted(
@Nullable AuthorizationServiceConfiguration fetchedConfiguration,
@Nullable AuthorizationException ex) {
if (ex != null) {
pendingFlow.compareAndSet(flow, null);
promise.reject("service_configuration_fetch_error", ex.getLocalizedMessage(), ex);
return;
}
Expand All @@ -312,10 +333,13 @@ public void onFetchConfigurationCompleted(
usePKCE,
additionalParametersMap,
androidTrustedWebActivity,
androidPrefersEphemeralSession);
androidPrefersEphemeralSession,
flow);
} catch (ActivityNotFoundException e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("browser_not_found", e.getMessage());
} catch (Exception e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("authentication_failed", e.getMessage());
}
}
Expand Down Expand Up @@ -352,10 +376,6 @@ public void refresh(
additionalParametersMap.put("client_secret", clientSecret);
}

// store setting in private field for later use in onActivityResult handler
this.dangerouslyAllowInsecureHttpRequests = dangerouslyAllowInsecureHttpRequests;
this.additionalParametersMap = additionalParametersMap;

// when serviceConfiguration is provided, we don't need to hit up the OpenID
// well-known id endpoint
if (serviceConfiguration != null || hasServiceConfiguration(issuer)) {
Expand Down Expand Up @@ -433,7 +453,11 @@ public void logout(
dangerouslyAllowInsecureHttpRequests, androidAllowCustomBrowsers);
final HashMap<String, String> additionalParametersMap = MapUtil.readableMapToHashMap(additionalParameters);

this.promise = promise;
final PendingFlow flow = new PendingFlow(promise, 53, null, null, null, null, false);
if (!pendingFlow.compareAndSet(null, flow)) {
promise.reject("authentication_in_progress", "Another authorization or logout is already in progress");
return;
}

if (serviceConfiguration != null || hasServiceConfiguration(issuer)) {
try {
Expand All @@ -447,8 +471,10 @@ public void logout(
postLogoutRedirectUri,
additionalParametersMap);
} catch (ActivityNotFoundException e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("browser_not_found", e.getMessage());
} catch (Exception e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("end_session_failed", e.getMessage());
}
} else {
Expand All @@ -460,6 +486,7 @@ public void onFetchConfigurationCompleted(
@Nullable AuthorizationServiceConfiguration fetchedConfiguration,
@Nullable AuthorizationException ex) {
if (ex != null) {
pendingFlow.compareAndSet(flow, null);
promise.reject("service_configuration_fetch_error", ex.getLocalizedMessage(), ex);
return;
}
Expand All @@ -474,8 +501,10 @@ public void onFetchConfigurationCompleted(
postLogoutRedirectUri,
additionalParametersMap);
} catch (ActivityNotFoundException e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("browser_not_found", e.getMessage());
} catch (Exception e) {
pendingFlow.compareAndSet(flow, null);
promise.reject("end_session_failed", e.getMessage());
}
}
Expand All @@ -489,112 +518,67 @@ public void onFetchConfigurationCompleted(
*/
@Override
public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) {
final PendingFlow flow = pendingFlow.get();
if (flow == null || flow.requestCode != requestCode || !pendingFlow.compareAndSet(flow, null)) {
return;
}

final Promise promise = flow.promise;
final String errorCode = requestCode == 52 ? "authentication_error" : "end_session_failed";
try {
if (requestCode == 52) {
if (data == null) {
if (promise != null) {
promise.reject("authentication_error", "Data intent is null" );
}
promise.reject(errorCode, "Data intent is null");
return;
}

final AuthorizationResponse response = AuthorizationResponse.fromIntent(data);
AuthorizationException ex = AuthorizationException.fromIntent(data);
if (ex != null) {
if (promise != null) {
handleAuthorizationException("authentication_error", ex, promise);
}
handleAuthorizationException(errorCode, ex, promise);
return;
}

if (this.skipCodeExchange != null && this.skipCodeExchange) {
WritableMap map;
if (this.usePKCE != null && this.usePKCE && this.codeVerifier != null) {
map = TokenResponseFactory.authorizationCodeResponseToMap(response, this.codeVerifier);
} else {
map = TokenResponseFactory.authorizationResponseToMap(response);
}

if (promise != null) {
promise.resolve(map);
}
if (requestCode == 53) {
promise.resolve(EndSessionResponseFactory.endSessionResponseToMap(EndSessionResponse.fromIntent(data)));
return;
}


final Promise authorizePromise = this.promise;
final AppAuthConfiguration configuration = createAppAuthConfiguration(
createConnectionBuilder(this.dangerouslyAllowInsecureHttpRequests, this.tokenRequestHeaders),
this.dangerouslyAllowInsecureHttpRequests,
null
);

AuthorizationService authService = new AuthorizationService(this.reactContext, configuration);

TokenRequest tokenRequest;
if(this.additionalParametersMap == null) {
tokenRequest = response.createTokenExchangeRequest();
} else {
tokenRequest = response.createTokenExchangeRequest(this.additionalParametersMap);
final AuthorizationResponse response = AuthorizationResponse.fromIntent(data);
if (response == null) {
promise.reject(errorCode, "Authorization response is missing");
return;
}
if (flow.skipCodeExchange) {
promise.resolve(flow.codeVerifier != null
? TokenResponseFactory.authorizationCodeResponseToMap(response, flow.codeVerifier)
: TokenResponseFactory.authorizationResponseToMap(response));
return;
}

AuthorizationService.TokenResponseCallback tokenResponseCallback = new AuthorizationService.TokenResponseCallback() {

AuthorizationService authService = new AuthorizationService(this.reactContext, flow.tokenConfiguration);
TokenRequest tokenRequest = flow.additionalParameters == null
? response.createTokenExchangeRequest()
: response.createTokenExchangeRequest(flow.additionalParameters);
AuthorizationService.TokenResponseCallback callback = new AuthorizationService.TokenResponseCallback() {
@Override
public void onTokenRequestCompleted(
TokenResponse resp, AuthorizationException ex) {
public void onTokenRequestCompleted(TokenResponse resp, AuthorizationException ex) {
if (resp != null) {
WritableMap map = TokenResponseFactory.tokenResponseToMap(resp, response);
if (authorizePromise != null) {
authorizePromise.resolve(map);
}
promise.resolve(TokenResponseFactory.tokenResponseToMap(resp, response));
} else {
if (promise != null) {
handleAuthorizationException("token_exchange_failed", ex, promise);
}
handleAuthorizationException("token_exchange_failed", ex, promise);
}
}
};

if (this.clientSecret != null) {
ClientAuthentication clientAuth = this.getClientAuthentication(this.clientSecret, this.clientAuthMethod);
authService.performTokenRequest(tokenRequest, clientAuth, tokenResponseCallback);

if (flow.clientSecret != null) {
authService.performTokenRequest(tokenRequest,
getClientAuthentication(flow.clientSecret, flow.clientAuthMethod), callback);
} else {
authService.performTokenRequest(tokenRequest, tokenResponseCallback);
}

} // close if

if (requestCode == 53) {
if (data == null) {
if (promise != null) {
promise.reject("end_session_failed", "Data intent is null" );
}
return;
}
EndSessionResponse response = EndSessionResponse.fromIntent(data);
AuthorizationException ex = AuthorizationException.fromIntent(data);
if (ex != null) {
if (promise != null) {
handleAuthorizationException("end_session_failed", ex, promise);
}
return;
}
final Promise endSessionPromise = this.promise;
if (endSessionPromise != null) {
WritableMap map = EndSessionResponseFactory.endSessionResponseToMap(response);
endSessionPromise.resolve(map);
authService.performTokenRequest(tokenRequest, callback);
}
}
} catch (Exception e) {
if(promise != null) {
} catch (Exception e) {
promise.reject("run_time_exception", e.getMessage());
} else {
throw e;
}
}
}

/*
* Perform dynamic client registration with the provided configuration
Expand Down Expand Up @@ -665,7 +649,8 @@ private void authorizeWithConfiguration(
final Boolean usePKCE,
final Map<String, String> additionalParametersMap,
final Boolean androidTrustedWebActivity,
final Boolean androidPrefersEphemeralSession) {
final Boolean androidPrefersEphemeralSession,
final PendingFlow flow) {

String scopesString = null;

Expand Down Expand Up @@ -726,8 +711,8 @@ private void authorizeWithConfiguration(
if (!usePKCE) {
authRequestBuilder.setCodeVerifier(null);
} else {
this.codeVerifier = CodeVerifierUtil.generateRandomCodeVerifier();
authRequestBuilder.setCodeVerifier(this.codeVerifier);
flow.codeVerifier = CodeVerifierUtil.generateRandomCodeVerifier();
authRequestBuilder.setCodeVerifier(flow.codeVerifier);
}

if (!useNonce) {
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-app-auth/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ type OAuthTokenErrorCode =
// https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError
type OICRegistrationErrorCode = 'invalid_redirect_uri' | 'invalid_client_metadata';
type AppAuthErrorCode =
| 'authentication_in_progress'
| 'service_configuration_fetch_error'
| 'authentication_failed'
| 'token_refresh_failed'
Expand Down