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

Make redirect_uri optional #947

Closed
Linuus opened this issue Mar 13, 2017 · 10 comments
Closed

Make redirect_uri optional #947

Linuus opened this issue Mar 13, 2017 · 10 comments

Comments

@Linuus
Copy link

Linuus commented Mar 13, 2017

Hi!

I have a question about the redirect_uri being required. I've seen a few other issues about this but they are all closed and I didn't find the answers that I need :)

Let's say someone is building an iOS application. They would use the "client_credentials flow" to authenticate against doorkeeper. When they create the app in doorkeeper, what would they put in the redirect_uri field? Isn't it weird to just add some dummy uri? Since they will never use "authorization code flow" the redirect uri will never be used.

Isn't it appropriate to just make it mandatory for authorization code flow requests? So, if I don't enter a redirect_uri for my app, I can't use the "authorization code flow"? It's not like we need to redirect to any uri because of this, right?

I think Twitter is doing something similar on their callback url:

To restrict your application from using callbacks, leave this field blank.

Perhaps I'm missing something obvious here so please enlighten me :D

@kevinetore
Copy link

kevinetore commented Jun 8, 2017

Is there any progress on this issue? I'm building a client application that implements a Two-legged OAuth2 strategy. Because of the validation on the redirect URI I need to bypass this by adding a dummy URI like @Linuus mentioned but that seems a bit odd.

@nbulaj
Copy link
Member

nbulaj commented Feb 4, 2018

Hi @Linuus. Does it correspond to the OAuth2 RFC ? It is required to do some research on this issue ...

@nbulaj
Copy link
Member

nbulaj commented Feb 7, 2018

As I can see redirect_uri can be blank. So we need to see what we can do on Doorkeeper side

@Linuus
Copy link
Author

Linuus commented Feb 7, 2018

@nbulaj Sorry for the late reply. Yes it looks like it is optional :) Thank you for taking a look at this!

@nbulaj
Copy link
Member

nbulaj commented Mar 15, 2018

So I made some exploration about this issue. To implement this one it is required to:

  • allow nil values for redirect_uri on model level
  • remove NOT NULL constraint from oauth_applications table

If the first one is possible without any braking changes, the second one requires additional actions, especially for legacy projects with old versions of Doorkeeper.

Other "side-effect" is that developers can set different grant flows (client credentials, password, auth code), but create all the applications without redirect URI... So the behavior of the Doorkeeper would be abnormal. It this case it is required to do more on top of auth logic of the gem.

@nbulaj
Copy link
Member

nbulaj commented Mar 15, 2018

@kevinetore what kind of grant type do you use?

@bpieck
Copy link

bpieck commented Aug 1, 2018

@Linuus: This is just a side remark. But you should have a look into PKCE. iOS Apps totally should use the redirect flow. There even is a special mobile-App variation of authentication_code grant (with PKCE). That is created because you normally cannot really trust the client credentials of mobile apps.

My feeling: This here is no real issue. There are some cases, where redirect_uri should not be needed (when there is some trusted server, that really only needs client_credentials flow) - but those are really rare cases. Mobile apps - never should need client credentials flow. I think, if a mobile app needs that - this API already can be assumed as to be public.

@nbulaj
Copy link
Member

nbulaj commented Feb 5, 2019

I made some research and I think we can make Doorkeeper opionated enough. We can check configured grant types and if it's only client_credentials or password then we can allow blank redirect URI for applications. In other cases it must be required.

So the only case in such implementation is when developer after some time (1 year for example) of using just password flow will add an implicit grant type to configuration. In this case everything will work (old clients will have their tokens new or old without any problem), but no new application can be created with blank redirect URI.

WDYT @Linuus @bpieck ?

@Linuus
Copy link
Author

Linuus commented Feb 9, 2019

I haven’t thought about this issue in years and the project where I used doorkeeper is dead :) But, it sounds reasonable to me!

@nbulaj nbulaj closed this as completed in 4ac115d Apr 1, 2019
nbulaj added a commit that referenced this issue Apr 1, 2019
Fix #947: Allow blank redirect URI for URI-less grant flows
@nbulaj
Copy link
Member

nbulaj commented Apr 1, 2019

Finally this feature released: #1237

Doorkeeper automatically will allow to use blank redirect URIs in case server configured to use URI-less grant flows (client credentials, resource owner password). One thing you need to do is to remove NOT NULL constraint from redirect_uri columns for oauth_applications table.

[NOTE]: don't forget that if you will enable oauth grant flows that require redirect URI (like authorization code or implicit) - you applications automatically becomes invalid because they have a blank column. You wouldn't be able t create an application without redirect URI using Doorkeeper admin panel. So use this feature carefully.

Read more here: https://github.com/doorkeeper-gem/doorkeeper/wiki/Allow-blank-redirect-URI-for-Applications

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

No branches or pull requests

4 participants