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

Support manual service configuration #52

Merged
merged 22 commits into from
Feb 22, 2018

Conversation

jevakallio
Copy link
Contributor

@jevakallio jevakallio commented Feb 21, 2018

Fixes #23.

Adds support for manually configuring service endpoints and supporting client secrets for non-OIDC-compliant OAuth services.

JavaScript

  • Add serviceConfiguration to API
  • Add clientSecret to API
  • Support authorize
  • Support refresh
  • Support revoke
  • Refactor
  • Update and add tests

Android

  • Allow authorization with manual service configuration
  • Allow refresh with manual service configuration
  • Pass additionalProperties to token request to support client_secret
  • Refactor

iOS

  • Allow authorization with manual service configuration
  • Allow refresh with manual service configuration
  • Pass client_secret

Other

  • Update API documentation
  • Update TypeScript definitions
  • Add note/warning about using client secrets
  • Add example configuration for a manually configured service

With this, we should next be able to implement:

@jevakallio jevakallio changed the title [WIP] Support manual service configuration Support manual service configuration Feb 21, 2018
README.md Outdated
@@ -72,14 +72,20 @@ const result = await authorize(config);
This is your configuration object for the client. The config is passed into each of the methods
with optional overrides.

* **issuer** - (`string`) _REQUIRED_ the url of the auth server
* **issuer** - (`string`) base URI of the authentication server. If no `serviceConfiguration` (below) is not provided, issuer is a mandatory field, so that the configuration can be fetched from the issuer's [OIDC discovery endpoint](https://openid.net/specs/openid-connect-discovery-1_0.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

"If no serviceConfiguration (below) is not provided" - double negative 🤔

README.md Outdated
@@ -467,6 +481,42 @@ const refreshedState = await refresh(config, {
});
```

### Uber

Uber provides an OAuth 2.0 endpoint for logging in with a Uber user's credentiala. You'll need to first [create an Uber OAuth application here](https://developer.uber.com/docs/riders/guides/authentication/introduction).
Copy link
Contributor

Choose a reason for hiding this comment

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

Uber's credentials

additionalParametersMap
);
} catch (Exception e) {
promise.reject(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

In other parts of the app we've been more explicit with the error, e.g.

promise.reject("RNAppAuth Error", "Failed to authenticate", e);

import java.util.HashMap;

/**
* Created by formidable on 21/02/2018.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure these "Created by" comments are particularly useful.

index.js Outdated
(serviceConfiguration &&
typeof serviceConfiguration.authorizationEndpoint === 'string' &&
typeof serviceConfiguration.tokenEndpoint === 'string'),
'Config error: you must provide either an issue or a service endpoints'
Copy link
Contributor

Choose a reason for hiding this comment

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

"an issuer or a service endpoint"

index.js Outdated
invariant(
typeof issuer === 'string' ||
(serviceConfiguration && typeof serviceConfiguration.revocationEndpoint === 'string'),
'Config error: you must provide either an issue or a revocation endpoint'
Copy link
Contributor

Choose a reason for hiding this comment

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

issue__r__

@jevakallio
Copy link
Contributor Author

@kadikraman fixed review notes, thanks!

When you merge this, feel free to squash, the commit messages are not particularly useful.

@kadikraman kadikraman merged commit 2228b67 into master Feb 22, 2018
@kadikraman kadikraman deleted the feature/manual-service-configuration branch February 22, 2018 11:39
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