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

Add dynamic client registration #418

Merged
merged 4 commits into from
Jan 6, 2020
Merged

Add dynamic client registration #418

merged 4 commits into from
Jan 6, 2020

Conversation

jasonpaulos
Copy link
Contributor

@jasonpaulos jasonpaulos commented Nov 21, 2019

I've started using this library with Solid to authenticate against inrupt.net and solid.community, and since Solid is decentralized it requires apps to dynamically register with the provider. Both AppAuth-Android and AppAuth-iOS provide support for client registration, so I added the Javascript and native code required to take advantage of that. I would really like to get this accepted so that others don't have to start from scratch with this too!

Description

I've added a new library function, register, which can be used to obtain a client id and secret for subsequent calls to authorize and other library functions.

I've tested the functionality on iOS and Android, and everything is working properly.

I did end up modifying the eslint settings, since it was giving me errors when using == null to check if parameters had been passed to register, and since it was limiting the number of statements that could be used in an arrow function, which affected the tests.

Steps to verify

import { register } from 'react-native-app-auth';

const res = await register({
  issuer: 'https://inrupt.net',
  redirectUrls: ['com.myapp.id:/oauthredirect']
});
// res is now { clientId: '...', clientSecret: '...', ... }

Let me know if there's anything I can do to help get this approved.

Copy link
Contributor

@kadikraman kadikraman left a comment

Choose a reason for hiding this comment

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

Thanks so much for the PR, this is great! Just a small update before I can merge it. We've recently updated the error messaging descriptions to make them consistent across platforms, so it would e great if we could update the errors in this PR before merging it

service_configuration_fetch_error - could not fetch the service configuration
authentication_failed - user authentication failed
token_refresh_failed - could not exchange the refresh token for a new JWT
registration_failed (new) - could not register

android/src/main/java/com/rnappauth/RNAppAuthModule.java Outdated Show resolved Hide resolved
android/src/main/java/com/rnappauth/RNAppAuthModule.java Outdated Show resolved Hide resolved
android/src/main/java/com/rnappauth/RNAppAuthModule.java Outdated Show resolved Hide resolved
ios/RNAppAuth.m Outdated Show resolved Hide resolved
ios/RNAppAuth.m Outdated Show resolved Hide resolved
@jasonpaulos
Copy link
Contributor Author

Thank you @kadikraman! I have updated the error messages as you suggested.

Copy link
Contributor

@kadikraman kadikraman left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kadikraman kadikraman merged commit 1396514 into FormidableLabs:master Jan 6, 2020
@kadikraman
Copy link
Contributor

Released in v5.0.0 🎉

@jasonpaulos jasonpaulos deleted the registration branch January 10, 2020 21:14
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