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

docs: Added Keycloak auth configuration #29487

Merged
merged 7 commits into from
Jul 11, 2024
Merged

Conversation

lindner-tj
Copy link
Contributor

SUMMARY

Was having issues enabling SSO using Keycloak, got to solution editing what is proposed here: #13806

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

superset_PR

ADDITIONAL INFORMATION

  • Has associated issue: how to configure superset with keycloak #13806
  • Required feature flags:
  • Changes UI: in documentation
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@github-actions github-actions bot added the doc Namespace | Anything related to documentation label Jul 4, 2024
@dosubot dosubot bot added the authentication:sso Single Sign On label Jul 4, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

@SnakebiteEF2000
Copy link

Is Related to this

@rusackas rusackas requested review from sfirke and dpgaspar July 7, 2024 05:07
@sfirke
Copy link
Member

sfirke commented Jul 8, 2024

There are often people asking questions about using Keycloak so this is a valuable addition, thank you!

Superset uses Flask-AppBuilder for authentication. My first instinct is that anything generic to FAB should go in its security documentation. In fact I see there is a Keycloak section already, I would hate to have anything duplicated between these two places as users will look to both. I'm talking about this: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/docs/security.rst#authentication-oauth

Then if there's anything specific to Superset, it should go in the Superset docs. What do you think @lindner-tj ? FYI @dpgaspar works on Preset but he also is the maintainer of FAB, which positions this well to coordinate across the two projects.

@lindner-tj
Copy link
Contributor Author

lindner-tj commented Jul 9, 2024

There are often people asking questions about using Keycloak so this is a valuable addition, thank you!

Superset uses Flask-AppBuilder for authentication. My first instinct is that anything generic to FAB should go in its security documentation. In fact I see there is a Keycloak section already, I would hate to have anything duplicated between these two places as users will look to both. I'm talking about this: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/docs/security.rst#authentication-oauth

Then if there's anything specific to Superset, it should go in the Superset docs. What do you think @lindner-tj ? FYI @dpgaspar works on Preset but he also is the maintainer of FAB, which positions this well to coordinate across the two projects.

Hi @sfirke,

when I was trying to set up SSO with Authlib, the issue I was facing wasn't caused by the Python config section. It seemed to originate from the CustomSsoSecurityManager file in the Superset Documentation. I was getting weird issues relating to "userDetails". I tried different URL endpoints but couldn't get it to work.
The solution I eventually found on the linked old GitHub thread doesn't change anything in FAB, it just uses a different OIDC provider. So in my mind it changes Superset and not FAB. This is also why I think adding a Flask-OIDC sub-section to the Superset docs is better than adding it to the FAB docs.

Edit: I understand that custom Security Managers are extensions to FAB, but when you're already providing an example for an Authlib Security Manager in the Superset docs, why not provide another one for Flask-OIDC?

Copy link
Member

@sfirke sfirke left a comment

Choose a reason for hiding this comment

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

Thanks @lindner-tj for your comment, I agree upon review it belongs here. I've added minor suggestions and have two questions.

docs/docs/configuration/configuring-superset.mdx Outdated Show resolved Hide resolved
docs/docs/configuration/configuring-superset.mdx Outdated Show resolved Hide resolved
AUTH_USER_REGISTRATION = True

# The default user self registration role
AUTH_USER_REGISTRATION_ROLE = 'Public'
Copy link
Member

Choose a reason for hiding this comment

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

Is this required for the OIDC integration to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't tested it without, but you are right, it most likely is not.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. It would be ideal if someone could test, then we could confirm whether this can be removed.

That said, it's not worth holding up the PR merge for.

Copy link
Contributor Author

@lindner-tj lindner-tj Jul 11, 2024

Choose a reason for hiding this comment

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

I removed the lines and it still works. I can still login via SSO and new users who were never logged-in before get registered. Oddly enough, new users still get assigned the 'Gamma' role. Note: Instead of 'Public' I had set it to AUTH_USER_REGISTRATION_ROLE = 'Gamma' in my setup.

docs/docs/configuration/configuring-superset.mdx Outdated Show resolved Hide resolved
docs/docs/configuration/configuring-superset.mdx Outdated Show resolved Hide resolved
@sfirke sfirke changed the title [docs] Added Keycloak configuration part to documentation docs: Added Keycloak auth configuration Jul 10, 2024
@sfirke
Copy link
Member

sfirke commented Jul 10, 2024

I'm good with merging this -- though it would be ideal if you could test whether that line about default role is needed -- but also it's failing pre-commit check. Looks like due to trailing whitespace. Can you fix that so it passes CI?

@kormpakis
Copy link

Would it be possible to post the steps needed to integrate Apache Superset with Keycloak one-by-one?
I really try to figure it out, but when I login with Keycloak is says that "The request to sign in was denied."

StackOverflow: https://stackoverflow.com/questions/78699787/apache-superset-keycloak-integration
GitHub issues: #29539

@lindner-tj
Copy link
Contributor Author

I'm good with merging this -- though it would be ideal if you could test whether that line about default role is needed -- but also it's failing pre-commit check. Looks like due to trailing whitespace. Can you fix that so it passes CI?

I removed the whitespace and made a new commit (6261e2b). Sorry for the ambiguous commit message.

@lindner-tj
Copy link
Contributor Author

lindner-tj commented Jul 11, 2024

Would it be possible to post the steps needed to integrate Apache Superset with Keycloak one-by-one? I really try to figure it out, but when I login with Keycloak is says that "The request to sign in was denied."

StackOverflow: https://stackoverflow.com/questions/78699787/apache-superset-keycloak-integration GitHub issues: #29539

Hi @kormpakis

Here is a step-by-step summary of what I did. Mind you I am using Docker compose to run my superset instance.

  1. in your /apache-superset/docker/requirements-local.txt File add the lines
Flask-OpenID
flask-oidc
  1. Create a new file named keycloak_security_manager.py in the same dir as superset_config.py.
  2. Add the lines from my doc addition to superset_config.py (Maybe include AUTH_USER_REGISTRATION = True and
    AUTH_USER_REGISTRATION_ROLE = 'Gamma' to make sure new users get registered.)
  3. In the same dir as superset_config.py create client_secret.json containing your Keycloak setup.

That's all I did.

Answering the question of @kormpakis made me aware that I forgot to include this.
@sfirke sfirke merged commit 3f6b7e2 into apache:master Jul 11, 2024
34 checks passed
@sfirke
Copy link
Member

sfirke commented Jul 11, 2024

thanks for the contribution @lindner-tj !

@kormpakis
Copy link

Would it be possible to post the steps needed to integrate Apache Superset with Keycloak one-by-one? I really try to figure it out, but when I login with Keycloak is says that "The request to sign in was denied."
StackOverflow: https://stackoverflow.com/questions/78699787/apache-superset-keycloak-integration GitHub issues: #29539

Hi @kormpakis

Here is a step-by-step summary of what I did. Mind you I am using Docker compose to run my superset instance.

  1. in your /apache-superset/docker/requirements-local.txt File add the lines
Flask-OpenID
flask-oidc
  1. Create a new file named keycloak_security_manager.py in the same dir as superset_config.py.
  2. Add the lines from my doc addition to superset_config.py (Maybe include AUTH_USER_REGISTRATION = True and
    AUTH_USER_REGISTRATION_ROLE = 'Gamma' to make sure new users get registered.)
  3. In the same dir as superset_config.py create client_secret.json containing your Keycloak setup.

That's all I did.

Dear @lindner-tj ,
Thanks a lot for your kind contribution. I followed your instructions, which were straightforward and clear - great appreciated!

One last thing, can you share the needed Keycloak client config? Because when I open Apache Superset, I do get redirected in a Keycloak environment, but this error appears: Invalid parameter: redirect_uri

For reference, in my Keycloak client, I have the following:
Valid redirect URIs: http://localhost:8089/oauth-authorized/keycloak
Web origins: http://localhost:8089/*

The same redirect uri is also in the client config:
"redirect_uris": [
"http://localhost:8089/oauth-authorized/keycloak"
],

I really pray for this to be the final thing to be resolved! :P

Thanks a lot again in advance!

@lindner-tj
Copy link
Contributor Author

Hi @kormpakis

{
    "mySsoProviderInsideKeycloak": {
        "issuer": ...
        ...
        "redirect_uris": [
            "https://myDnsAlias/oauth-authorized/mySsoProviderInsideKeycloak"
        ],
        ...

hope this helps. The endpoint of the redirect_uri needs to match the word before the curly bracket in line 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication:sso Single Sign On doc Namespace | Anything related to documentation size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants