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 Hybrid Flow #75

Open
Tylerflick opened this Issue May 4, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@Tylerflick
Copy link

Tylerflick commented May 4, 2016

Are there any plans to support the Hybrid Flow described in section 2 of http://openid.net/specs/openid-connect-implicit-1_0.html?

@ve7jtb

This comment has been minimized.

Copy link
Collaborator

ve7jtb commented May 4, 2016

I think supporting "code id_token" may warrant discussion. I don't know that it would be my highest priority.

@ve7jtb ve7jtb added invalid question and removed invalid labels May 4, 2016

@iainmcgin

This comment has been minimized.

Copy link
Member

iainmcgin commented May 4, 2016

In order to be able to handle implicit flows in the library, the following would be required:

  1. An app link registration in the app to ensure that only the authorized app can receive the redirect. This requires Android M (v23) and up; on prior versions of Android, it is possible to register an intent handler for certain URLs, but not securely.
  2. Parsing of parameters carried in the fragment. This isn't particularly difficult, but I haven't checked to see if the browser will actually hand over the fragment to the app for us to do this.

A stop-gap solution would be to set the redirect URI to a page that can parse out the fragment values and forward them to the app via a custom scheme. Without PKCE this is still vulnerable, however.

@Tylerflick

This comment has been minimized.

Copy link
Author

Tylerflick commented May 20, 2016

Quick update. I ended up wrapping the android Uri class using a composition style pattern. The new Uri class internally takes care of parsing the either the fragment or query parameters. This has ended up working as a drop in replacement with no issues so far.

@iainmcgin

This comment has been minimized.

Copy link
Member

iainmcgin commented May 20, 2016

Excellent work! So, it sounds like you get the fragment from the browser no problem, so I should open an issue to support parsing the response out of the fragment in RedirectUriReceiverActivity. It's good to know you were able to solve your problem without having to change the library too.

@Tylerflick

This comment has been minimized.

Copy link
Author

Tylerflick commented May 20, 2016

Unfortunately I did have to make a few small changes to the library. The Uri wrapping class had to replace the original android.net.Uri class in :

  • AdditionalParamsProcessor.java
  • AuthorizationResponse.java
  • RedirectUriReceiverActivity.java

The wrapping class keeps the same API though so it was more of just swapping out of the types in the appropriate method signatures.

If you would like I can make a fork of the library and push the changes.

@iainmcgin

This comment has been minimized.

Copy link
Member

iainmcgin commented May 21, 2016

Ah, I understand. So, it sounds like I should just make the change in RedirectUriReceiverActivity to support extracting the response from the fragment, to avoid a more complex solution of priming separate activities for query vs fragment responses.

@iainmcgin iainmcgin added this to the 1.0.0 milestone May 21, 2016

@iainmcgin iainmcgin self-assigned this May 21, 2016

@Tylerflick

This comment has been minimized.

Copy link
Author

Tylerflick commented May 23, 2016

Just pushed my proof of concept here.

@ve7jtb

This comment has been minimized.

Copy link
Collaborator

ve7jtb commented Apr 19, 2017

using a app link is really only required if you are directly returning token.

That we should discourage people from doing.

For code id_token a custom scheme redirect would work, with the caveat that the id_token could leak. The problem with leaking the id_token is if the app is passing the id_token to a backend for authentication (As google recommends in some cases) then you get into trouble.

Is there really a use case for something other than code response type?

While the hybrid response modes (they are not implicit) like code id_token are possible they may create security issues as the responses are not protected by PKCE.

John B.

@User23846923

This comment has been minimized.

Copy link

User23846923 commented Feb 7, 2019

Hi. Can I ask for some clarification please? Is hybrid flow i.e. "code id_token" with PKCE and a custom scheme supported or not? It is a recommended flow in the IdentityServer docs, but it doesn't appear to work, and this issue just ends with the rhetorical question above.

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