Skip to content

[WIP] [Question] Add a token to badges for private repos#286

Open
valeriangalliat wants to merge 4 commits into
alanshaw:masterfrom
busbud:feature/private-repo-badge-token
Open

[WIP] [Question] Add a token to badges for private repos#286
valeriangalliat wants to merge 4 commits into
alanshaw:masterfrom
busbud:feature/private-repo-badge-token

Conversation

@valeriangalliat

@valeriangalliat valeriangalliat commented May 16, 2016

Copy link
Copy Markdown

Issue

We can't share/embed David badges for private repos. Currently, viewing the badge for a private repo requires to be logged-in with a GitHub account that have access to the repo in question.

  1. It wouldn't be a good idea to make the badges for private repos public.
  2. We can't even do this technically because if the repo data is not cached or out of date, we need the GitHub token to access the private repo in order to refresh the badge.
  3. Since David is "stateless" (the database is wiped on boot), there's no way to reliably persist and get the GitHub token for a private repo without being actually logged-in on a GitHub account with access to this repo.
  4. Passing the GitHub token as parameter is out of question since it would give direct access to the user info and repos.

Closes #165, #176.

Proposition

For private repos, add a token parameter to the badge URL. This token is the current user's GitHub token, encrypted with a secret server key provided via configuration.

Pros

  • Stateless: doesn't require any persistent data store to keep track of tokens.
  • Secure: only David with the secret key can get back the GitHub token from the encrypted parameter.

Cons

  • Can't revoke a token directly. If you give a badge URL with a token to someone, the badge will always be accessible and cannot be revoked unless the GitHub OAuth token itself is revoked, or the encryption secret is changed for the whole site.

Implementation

Config

  • Add token.algorithm and token.secret config options, algorithm defaulting to aes-256-ctr (which is I think a sane default), and no secret. If no secret is set, no token will be generated and the behavior will be the same as now.

Library

  • Add lib/badge-token.js, exposing encrypt and decrypt functions, using the config algorithm and secret, and getAuthToken to get a token either from the current session or the globally configured GitHub token. The encrypt function appends an HMAC to the generated token, and the decrypt function verifies it.
  • Modify the manifest to ensure the package.json private key is always set (if not explicitly set, will be whether the GitHub repo is private). Allows the rest of the code to do specific actions for private repos.
  • Also modify the manifest to always get repo status (to know whether it's private); indeed, even if there's no session auth token, there can be a global GitHub token configured that gives access to private repos.

Index

  • Inject badgeToken in badge and status routes.

Frontend

  • Modify embed-badge* views to take qs and linkQs variables instead of path. This is necessary because we need to add a token parameter for private repos, but since there's already a potential path parameter, it's more complex to add a new parameter (might need a ? or a &), and I prefered to rely on jQuery.params and give the whole query string. But since the token is only for the image and not the link, I introduced two variables for this.
  • In the status page, add a data-token with generated token for the badge of private repos.
  • Also modify the script of the status page to pass the new query string variables to the embed-badge* views, using jQuery.params.

Routes

  • In the badge route, support token parameter, decrypt it to get the GitHub token and use it in place of the session token if valid.
  • In the status route, for a private repo, generate the encrypted token (from session or global GitHub token, using badgeToken.getAuthToken) and pass it to the view.

To-do

  • I'm thinking it can be a good idea to include the repo name together with the token before encrypting, and checking it matches the requested repo when requested, this way a token would be unique per GitHub token and repo.

Note: this PR pretty critical since it involves encryption and security of GitHub tokens. I'm not a cryptography expert, so if we end up going with this solution, I'd feel better having it thoroughly reviewed by someone competent, and it would maybe we wise to ping the GitHub security team at some point.

@valeriangalliat valeriangalliat changed the title Add a token to badges for private repos [WIP] [Question] Add a token to badges for private repos May 16, 2016
@alanshaw

Copy link
Copy Markdown
Owner

Hey @valeriangalliat, this is amazing! Thank you for taking the time to work on this. I'm going to review and merge as soon as I can, sorry I haven't had a chance to yet! :)

@sbolel

sbolel commented Jun 25, 2016

Copy link
Copy Markdown

Hi @alanshaw, any updates on this? This would be awesome! Thanks @valeriangalliat for putting in this work!

@joshwiens

Copy link
Copy Markdown

@alanshaw - Same general question. It would be awesome to be able to use this in my private "work related" repos as well as the public ones.

@rmharrison rmharrison mentioned this pull request Sep 24, 2016
@hiradyazdan

Copy link
Copy Markdown

@alanshaw likewise, I am interested to know if there's any update on this PR. Thanks all

@devacto

devacto commented Nov 17, 2016

Copy link
Copy Markdown

@alanshaw Would you please let us know the status on this? Thank you very much for making david-dm. It has helped us a lot! 😃

@rmharrison

Copy link
Copy Markdown

Any status update?...or a reason the PR was approved, but then went stale before merge?

@fernandogmar

Copy link
Copy Markdown

any news here?

@alexdevero

Copy link
Copy Markdown

Is there any progress or update on this feature?

@teeleek

teeleek commented Sep 21, 2017 via email

Copy link
Copy Markdown

@johncade

johncade commented Aug 9, 2018

Copy link
Copy Markdown

Any update on the status of this? Was it resolved?

@cfanoulis

Copy link
Copy Markdown

is there ANTHING that is going to be done with this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.