Skip to content

Add support for OneLogin - #200

Merged
wolfeidau merged 4 commits into
Versent:masterfrom
pavelnikolov:add-onelogin-support
Jul 25, 2018
Merged

Add support for OneLogin#200
wolfeidau merged 4 commits into
Versent:masterfrom
pavelnikolov:add-onelogin-support

Conversation

@pavelnikolov

@pavelnikolov pavelnikolov commented Jul 18, 2018

Copy link
Copy Markdown
Contributor

This PR adds OneLogin provider support. There are few things that would like to discuss. OneLogin is a bit different than other providers. In order to access their API, each application needs to have app_id, client_id, client_secret. Without these I cannot generate access_token, which is in turn used to generate SAML assertion and to verify MFA options.

This PR is very naive and adds the app_id into the common configuration and the when prompting for credentials I check if the provider is OneLogin and I validate if client_id and client_secret are provided. Ideally, I would like to refactor the library (i.e. change the signature of the providers) to allow each provider to request/prompt for additional configuration options and keep the common clean. Any feedback and comments would be highly appreciated.

Unit tests will be ready very soon.

@pavelnikolov
pavelnikolov force-pushed the add-onelogin-support branch from 91465fa to 33656db Compare July 18, 2018 03:11
@pavelnikolov
pavelnikolov force-pushed the add-onelogin-support branch from 33656db to 242063f Compare July 18, 2018 14:58
Comment thread pkg/flags/flags.go
SkipPrompt bool
SkipVerify bool
Profile string
Subdomain string

@pavelnikolov pavelnikolov Jul 18, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ideally, I don't want to put these 4 new fields there. They are only needed for OneLogin. Any ideas where to put them? I wanted to refactor the library and allow each provider to have custom config and secrets. Such change would make this PR huge so I would split changes and refactoring in different PRs.

@pavelnikolov

Copy link
Copy Markdown
Contributor Author

This PR solves #121

@wolfeidau

Copy link
Copy Markdown
Contributor

So as it turns out I am going to add those attributes for another provider soon to support openid based login.

Although this is a big change and you have touched a bunch of things I do love getting new providers so happy to take it as is.

Cheers for the work you have done!

@wolfeidau
wolfeidau merged commit ac318a6 into Versent:master Jul 25, 2018
@btsteve

btsteve commented Jul 27, 2018

Copy link
Copy Markdown

Is there an example of what need to go in each field. For Subdomain am i adding just the host part or the full url? Where do i get the client_id, secret, and app id

@pavelnikolov

Copy link
Copy Markdown
Contributor Author

Subdomain is your organisation unique name. It is usually the subdomain part of the One Login dashboard URL.

Please, follow this tutorial to generate client credentials https://developers.onelogin.com/api-docs/1/getting-started/working-with-api-credentials

@pavelnikolov
pavelnikolov deleted the add-onelogin-support branch August 9, 2018 23:50
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.

3 participants