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

Support authentication through Github Enterprise #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

csabakoncz
Copy link

config.yml is consulted for various Github URLs so that
Jingo can connect to Github instances hosted at arbitrary locations.

E.g. to login using https://mygithub.com one can add the following
configuration (tested with Github Enterprise 2.6):

github:
enabled: true
redirectURL: ''
clientId:
clientSecret:
authorizationURL: https://mygithub.com/login/oauth/authorize
tokenURL: https://mygithub.com/login/oauth/access_token
userProfileURL: https://mygithub.com/api/v3/user

config.yml is consulted for various Github URLs so that
Jingo can connect to Github instances hosted at arbitrary locations.

E.g. to login using https://mygithub.com one can add the following
configuration (tested with Github Enterprise 2.6):

  github:
    enabled: true
    redirectURL: ''
    clientId: <clientid>
    clientSecret: <clientsecret>
    authorizationURL: https://mygithub.com/login/oauth/authorize
    tokenURL: https://mygithub.com/login/oauth/access_token
    userProfileURL:  https://mygithub.com/api/v3/user
callbackURL: redirectURL,
authorizationURL: auth.github.authorizationURL,
tokenURL: auth.github.tokenURL,
userProfileURL: auth.github.userProfileURL
Copy link
Owner

Choose a reason for hiding this comment

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

What if those values are not provided in the config.yaml (they'll be set as undefined and this would probably cause issues)?

The config.js needs also to be updated to provide default values (empty strings?) for those.

Also, we could test for these values to be present before using them.

And finally, any configuration option needs also to be referenced in the README file.

Thanks :)

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.

None yet

2 participants