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

How should OAuth Client IDs be distributed to headless apps? #1

Open
alecbcs opened this issue Dec 27, 2020 · 6 comments
Open

How should OAuth Client IDs be distributed to headless apps? #1

alecbcs opened this issue Dec 27, 2020 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@alecbcs
Copy link

alecbcs commented Dec 27, 2020

Hi, I apologize if I've just missed something in the documentation but, should our OAuth App Client IDs be kept secret? In the case of writing a headless app how should we distribute the Client IDs to the user's applications so that they can authenticate with GitHub? Is it best to write a server side relay that requests the tokens on the behalf of the CLI tool and then passes the resulting temporary code and token back to the user, so that the user never has access to the Client ID, or is it safe to embed the client ID in a distributed binary/source code? Thanks for your time.

@mislav
Copy link
Contributor

mislav commented Jan 5, 2021

(I'm guessing that you're talking about OAuth secrets. The client ID for any OAuth app is public knowledge.)

That's a great question! The short answer: just embed the "secret" value in your app. Even while compiled/obfuscated, assume that the value will always be trivially extractable from your app (and thus not a secret anymore), but that's fine. We do this in GitHub CLI.

Basically, the concept of an OAuth secret doesn't translate well to client apps, since it was designed with web applications in mind. For their OAuth applications, for example, Google asks each developer to indicate whether an OAuth app is a client app and doesn't provide nor require secrets for those apps, since they cannot possibly remain secret. The rest of the protocol remains the same.

Naturally, all this means that anyone can take your OAuth client ID + secret and pretend that they are your application. That's true, but it isn't such a big risk as it sounds. By the nature of running on the user's machine, client apps are not guaranteed to be tamper-proof anyway.

I guess we should add some docs about this to the README.

@mislav mislav added the documentation Improvements or additions to documentation label Jan 5, 2021
@alecbcs
Copy link
Author

alecbcs commented Jan 6, 2021

@mislav Thanks for taking the time to write a well detailed answer and explanation. I'm glad to hear that I wasn't missing something simple and this is a design discrepancy between web and client side apps. Also that leaking the client ID and the secret doesn't open up a major vulnerability besides securing that clients are using an authentic app and not an imposter.

@ghost

This comment has been minimized.

@jvalkeal
Copy link

jvalkeal commented Feb 4, 2022

Just wanted to leave a note here as I was looking how gh cli does these things and wondered why it has a secret exposed in a source tree with a comment:

https://github.com/cli/cli/blob/4f0264e37b2b42253ea866f07b010e5b840aa416/internal/authflow/flow.go#L21

Looks like if you use device-flow you don't need clientSecret anymore as api doesn't require its use thus you don't need to expose it. Thought you get back in same scenario @mislav wrote that you can use any clientId in your app.

@williammartin
Copy link
Member

This looks like it was addressed so I'm going to go ahead and close it. Thanks all.

@williammartin
Copy link
Member

Actually nevermind I see that the desired outcome from this was to update docs, and that hasn't happened, so seems reasonable to keep open.

@williammartin williammartin reopened this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants