Skip to content

[Java] Support custom ok-http authenticators#6261

Open
lwander wants to merge 1 commit intoswagger-api:masterfrom
lwander:custom-ok-http-gson-auth
Open

[Java] Support custom ok-http authenticators#6261
lwander wants to merge 1 commit intoswagger-api:masterfrom
lwander:custom-ok-http-gson-auth

Conversation

@lwander
Copy link
Copy Markdown
Contributor

@lwander lwander commented Aug 8, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Users who want to supply custom authenticators can subclass ApiClient and override getClientAuthenticators.

Waiting on #6256 to update petstore samples.

Users who want to supply custom authenticators can subclass `ApiClient`
and override `getClientAuthenticators`.
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 9, 2017

@lwander thanks for the PR.

cc @cbornet @jfiala @bbdouglas

@cbornet
Copy link
Copy Markdown
Contributor

cbornet commented Aug 9, 2017

Is subclassing the best way ? Why are the authentications unmodifiable in the first place ?

@lwander
Copy link
Copy Markdown
Contributor Author

lwander commented Aug 9, 2017

My guess had been that modifying the authentications during runtime could lead to some strange behavior, especially if removing something like an OAuth authenticator while a token was still alive. But I don't know the underlying reason. Maybe we can restrict modification to only adding authenticators?

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Oct 17, 2017

@lwander I got the same question as @cbornet. If you would like this change to proceed, please make the authentications modifiable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants