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

OpenID Connect and OAuth2 sign-on improvement #1112

Merged
merged 7 commits into from
Apr 6, 2020
Merged

OpenID Connect and OAuth2 sign-on improvement #1112

merged 7 commits into from
Apr 6, 2020

Conversation

1earch
Copy link

@1earch 1earch commented Sep 3, 2019

This patch fixes #1110.

Bug fixes

Improvements

  • SSO login now supports:
    • Groups parsing from user info (fixes the Invalid URL error)
    • Auto-login
    • Auto-updating user profile
  • Default config parameters are following OpenID conventions
  • The OAuth2 code is now removed from the URL after SSO signing in (fixes Authentication failure on disconnection)

In addition, I improved the login form when SSO signing in. Before this commit, nothing was displayed indicating that we were signing in using SSO. Now, when we are redirected to TheHive after signing on our federated identity provider, TheHive disables the login form and change button message to indicate that a SSO signing in is currently performed. The following image shows this feature:

image

Finally, a user with no matched roles (using the group mapper) can't SSO sign in. An AuthenticationError will be raised. This is needed in case of an internal user wants to access TheHive but should not: this user will match no profiles (read, write, admin).

PS: I will submit another PR on TheHiveDocs to explain how to configure SSO login.

User groups are parsed directly from user infos. The config
parameter `auth.sso.groups.url` is useless in that case.

Note: an AuthenticationError is raised when no groups are
available for the user.
Default now respects OpenID Connect conventions.
@1earch
Copy link
Author

1earch commented Sep 3, 2019

Something is drastrically changing, and may cause issues:

Before this PR, the OAuth2 authentication code was extracted from an URL following the pattern

https://hive-instance.com/index.html?code=REDACTED_CODE#!/login

Now, it is extracted from an URL following the pattern

https://hive-instance.com/index.html#!/login?code=REDACTED_CODE

@To-om To-om changed the base branch from master to develop September 3, 2019 13:33
Copy link
Contributor

@To-om To-om left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I have some comments on backend side. I let @nadouani review frontend code.

@To-om To-om requested a review from nadouani September 3, 2019 15:47
@1earch
Copy link
Author

1earch commented Sep 5, 2019

With the latest commit, the missing log message mentionned in #1010 will be rendered.

@To-om It's my first time with Scala. Please excuse me for not following best practices

@1earch 1earch requested a review from To-om September 6, 2019 14:51
@d3vzer0
Copy link

d3vzer0 commented Oct 22, 2019

@1earch I was struggling with the same issues using OAuth (OpenID). Getting generic "Authentication Failed" issues but can't seem to find any logs indicating what could be wrong. Hopefully this change will fix this ^^ Great addition regarding the auto update btw! Any update about the merge/review?

@ctII
Copy link

ctII commented Mar 3, 2020

Last comment on this was Oct 22, 2019. Is there any update on this?

@To-om To-om merged commit a0da8e3 into TheHive-Project:develop Apr 6, 2020
@1earch 1earch deleted the patch-openidconnect branch April 27, 2020 10:09
@1earch 1earch mentioned this pull request Apr 27, 2020
jiprocha added a commit to jiprocha/Cortex that referenced this pull request Dec 1, 2022
…ctionality

This commit fixes issue TheHive-Project#344 by backporting fix from TheHive repository.
Original pull request from which the backport was taken can be found at
TheHive-Project/TheHive#1112.
To-om pushed a commit to TheHive-Project/Cortex that referenced this pull request Sep 20, 2023
…ctionality

This commit fixes issue #344 by backporting fix from TheHive repository.
Original pull request from which the backport was taken can be found at
TheHive-Project/TheHive#1112.
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.

4 participants