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

add header authentication support for the web-ui #16558

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

Conversation

kenandominic
Copy link

@kenandominic kenandominic commented Mar 14, 2023

Description

Installs the header authenticator and header authenticator manager modules in the web UI authentication module

relevant configuration

web-ui.authentication.type=header

in place of #14450

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@kenandominic
Copy link
Author

kenandominic commented Mar 15, 2023

hey @lukasz-walkiewicz could you ptal when you get the chance. wrote tests as you asked in #14450

Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Jan 17, 2024
@mosabua
Copy link
Member

mosabua commented Jan 17, 2024

👋 @kenandominic - this PR has become inactive. We hope you are still interested in working on it. Please let us know, and we can try to get reviewers to help with that.

We're working on closing out old and inactive PRs, so if you're too busy or this has too many merge conflicts to be worth picking back up, we'll be making another pass to close it out in a few weeks.

@github-actions github-actions bot removed the stale label Jan 18, 2024
@huw0
Copy link
Member

huw0 commented Jan 22, 2024

I'm also interested in this feature. @hashhar @lukasz-walkiewicz what is outstanding to merge this?

Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Feb 12, 2024
@kenandominic
Copy link
Author

hey @mosabua, I'll spend some time again, hopefully later today, to resolve the merge conflicts if you can get someone to review it.

@kenandominic
Copy link
Author

hey @huw0, you can apply the changes in this PR if you have a fork of trino. we've (salesforce) been using header authn to guard our UI for nearly a year now

Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla-signed label Feb 12, 2024
Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

2 similar comments
Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@kenandominic
Copy link
Author

hey @mosabua I have resolved all the conflicts

@mosabua
Copy link
Member

mosabua commented Feb 12, 2024

@cla-bot check

Copy link

cla-bot bot commented Feb 12, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

Copy link

cla-bot bot commented Feb 12, 2024

The cla-bot has been summoned, and re-checked this pull request!

@mosabua
Copy link
Member

mosabua commented Feb 12, 2024

Can you submit a signed CLA and check the build failures out @kenandominic ?

Copy link

cla-bot bot commented Feb 13, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

1 similar comment
Copy link

cla-bot bot commented Feb 13, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Kenan Dominic.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@github-actions github-actions bot removed the stale label Feb 13, 2024
@kenandominic
Copy link
Author

hey @mosabua I was able to fix the issue with the cla check not passing. all tests pass except trino-delta-lake which is unrelated to the changes in this pr. is there a way to run just that check again?

@mosabua
Copy link
Member

mosabua commented Feb 21, 2024

Thanks for the CLA and such @kenandominic . The test results coming from the Delta Lake test are probably just a CI failure. Every new push will trigger the whole build again but I would wait until we get further review comments.

@mosabua
Copy link
Member

mosabua commented Feb 21, 2024

Can @lukasz-walkiewicz @hashhar or maybe @dain help here?

@huw0
Copy link
Member

huw0 commented Mar 4, 2024

hey @huw0, you can apply the changes in this PR if you have a fork of trino. we've (salesforce) been using header authn to guard our UI for nearly a year now

Great to hear that this has been battle tested!
So far I've been trying to avoid forking and sticking with upstream as closely as possible to reduce the overhead of staying up to date.


Is there anything that can be done to progress this PR?

Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Mar 26, 2024
Copy link

Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.

@github-actions github-actions bot closed this Apr 16, 2024
@mosabua
Copy link
Member

mosabua commented Apr 17, 2024

Reopening .. we still want this feature to get in. Adding stale-ignore label.

@mosabua mosabua reopened this Apr 17, 2024
@mosabua mosabua added stale-ignore Use this label on PRs that should be ignored by the stale bot so they are not flagged or closed. and removed stale labels Apr 17, 2024
@huw0
Copy link
Member

huw0 commented Jul 26, 2024

Hi @mosabua, @lukasz-walkiewicz - would be great to get this in soon if possible?

@mosabua
Copy link
Member

mosabua commented Aug 26, 2024

@wendigo @koszti .. you are working on this a bit recently. Any input?

Also @kenandominic can you change commit message to

"Add header authentication support to the Web UI"

@@ -46,6 +46,7 @@ The following Web UI authentication types are also supported:
- `KERBEROS`, see details in {doc}`/security/kerberos`
- `JWT`, see details in {doc}`/security/jwt`
- `OAUTH2`, see details in {doc}`/security/oauth2`
- `HEADER`, see details in {doc}`/develop/header-authenticator`
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to get user facing docs for header authentication going as well at some stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed docs stale-ignore Use this label on PRs that should be ignored by the stale bot so they are not flagged or closed.
Development

Successfully merging this pull request may close these issues.

3 participants