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

Set Discord to Prompt = None #605

Merged
merged 3 commits into from Sep 1, 2020
Merged

Conversation

anishanne
Copy link
Contributor

When you set the value of prompt to none in the get params, Discord will not prompt the user if they have auth'd with your app before. (It will just display the page and then the page disappears and they are in.)

Docs: https://discord.com/developers/docs/topics/oauth2#authorization-code-grant-authorization-url-example

@vercel
Copy link

vercel bot commented Aug 26, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/ojdi60ris
✅ Preview: https://next-auth-git-fork-anishanne-patch-1.nextauthjs.vercel.app

@vercel vercel bot temporarily deployed to Preview August 26, 2020 02:22 Inactive
@LoriKarikari
Copy link
Contributor

I think it's always better to provide a prompt so the user knows which data the application has access to. @iaincollins what are your thoughts about this one?

@nyedidikeke
Copy link
Contributor

@LoriKarikari I agree, but it will be great as a configurable option to the application owner.

@LoriKarikari
Copy link
Contributor

@nyedidikeke yes we should add an option to change this.

@LoriKarikari
Copy link
Contributor

@iaincollins I think we should leave the option as it is now but make it possible to change this value?

@anishanne
Copy link
Contributor Author

The way that the prompt works is the first time the user auths with the app, they have to give consent. Then every time after, they don't (provided the scopes are the same). It's basically cookie/session based login just if you have a Discord session it still works.

@LoriKarikari
Copy link
Contributor

In that case, it's fine to merge this PR! Thanks for the clarification @anishanne!

@vercel vercel bot temporarily deployed to Preview August 31, 2020 19:01 Inactive
Copy link
Contributor

@nyedidikeke nyedidikeke left a comment

Choose a reason for hiding this comment

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

@anishanne @LoriKarikari I don't think it's a good idea to change the current behaviour unless programmatically optional and easily configurable but should this be considered, let's not override #590 as Discordapp.com is now Discord.com.

@vercel vercel bot temporarily deployed to Preview August 31, 2020 21:34 Inactive
@anishanne
Copy link
Contributor Author

anishanne commented Aug 31, 2020

@nyedidikeke I think it would be better to not request users for consent each time they decide to login, if the scopes are the same & its the same user, just log them right in. However I would agree that making it configurable might be the best bet here.

Copy link
Contributor

@nyedidikeke nyedidikeke left a comment

Choose a reason for hiding this comment

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

The current no longer override #590 👍🏿

@LoriKarikari
Copy link
Contributor

I'm going to leave this one open and see what @iaincollins thinks about this.

@iaincollins
Copy link
Member

iaincollins commented Aug 31, 2020

@anishanne @LoriKarikari @nyedidikeke Thanks everyone!

I think prompt=none makes sense as that is how the other providers work, users shouldn't see a prompt on subsequent sign in attempts (unless there is a technical reason for that specific to Discord?).

Most providers with this way, though Google actually has the same option. By default they do not prompt after the initial sign in (unless you have multiple accounts, then sometimes you are asked to pick one). However with Google, you can request a prompt on sign in. Usually there are only some scenarios where a prompt is required and specific reasons for doing it.

e.g. In the case of Google it can be used to to issue a Refresh Token again - as, unlike other providers, they usually only provide one on initial sign in and you are expected to store it in a database - their intended use case for prompting and re-issuing one is mobile or desktop applications (that persist sessions forever, but need to get it again if the user uninstalls and reinstalls the app as unlike a web app they don't usually store it in an online database).

We can always add an example to the documentation for this for folks who might want to prompt if there is use case for it. If folks want to configure this they can always override the authorizationUrl option on the provider, and we could show how to do that in the docs.

I agree it seems like discord.com is the right domain to use.

@iaincollins
Copy link
Member

Thanks folks! Will merge in now and will go out in the next release!

@iaincollins iaincollins merged commit 08d7f5d into nextauthjs:main Sep 1, 2020
@iaincollins iaincollins added the enhancement New feature or request label Sep 1, 2020
@github-actions
Copy link

github-actions bot commented Feb 1, 2021

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants