-
Notifications
You must be signed in to change notification settings - Fork 28
feat(console): Make OAuth token URL configurable #136
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
base: main
Are you sure you want to change the base?
feat(console): Make OAuth token URL configurable #136
Conversation
.../src/test/java/org/apache/polaris/appruner/maven/mavenit/ITSimulatingTestUsingThePlugin.java
Show resolved
Hide resolved
binarycat0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution and for taking the time to work on this PR — it’s appreciated.
I noticed that this change includes several updates that seem unrelated or redundant with respect to the main purpose of the PR. To help keep reviews focused and maintainable, could you please reduce the scope to only the changes strictly required for the intended fix/feature?
Since this repository is maintained under the Apache Polaris community, we also try to follow the Apache contribution guidelines closely. In particular, it would be great if you could align this PR with the code contribution guidelines described here:
https://polaris.apache.org/community/contributing-guidelines/#code-contribution-guidelines
Narrowing the changeset and following these guidelines will make it much easier for the community to review and merge the contribution.
Thanks again for your effort, and please let us know if anything in the guidelines is unclear.
console/docker/Dockerfile
Outdated
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # regarding copyright ownership. The ASF licenses this file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace?
console/docker/Dockerfile
Outdated
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| # under the License. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace
console/docker/Dockerfile
Outdated
|
|
||
| # Build stage | ||
| FROM registry.access.redhat.com/ubi9/nodejs-22-minimal:9.7-1767673763 AS builder | ||
| FROM node:22-slim AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK this image was used in purpose.
Please check this PR - 34e63e6
console/docker/Dockerfile
Outdated
|
|
||
| # Copy package files | ||
| COPY package.json ./ | ||
| COPY package. json ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace
if i am not wrong please confirm that i have to Revert to Red Hat UBI image?? |
I consider this change as not meeting the main purpose of this PR. Besides that please review your changes, it contains many additional whitespaces and non ASCII chars, all around the codebase. |
I will work on this pr in the following week |
- Add AuthType type ('internal' | 'keycloak') to support multiple auth providers
- Update auth.ts to handle both internal Polaris and Keycloak authentication
- Add Keycloak proxy configuration in vite.config.ts for development
- Update login UI with authentication type selector
- Add conditional Keycloak realm field when using external auth
- Store auth type and realms in localStorage for session persistence
- Update .env.example with Keycloak configuration documentation
This allows users to choose between:
1. Internal Polaris authentication (default)
2. External Keycloak authentication
Fixes apache#98
0d8fefe to
bd52225
Compare
implementation proof, this actualy fixes Issue #98what Issue #98 asked for
what my previous pull request did wrong
what This pull request does right1. follows reference implementationThis matches @jbonofre's working implementation in the keycloak-support branch that was referenced in issue #98. 2. authentication type selectoruusers can now choose between:
3. dynamic url rroutingThe code now routes to different 0auth endpoints based on user selection: if (authType === "keycloak") {
if (!realm) {
throw new Error("Keycloak realm is required for Keycloak authentication")
}
tokenUrl =
} else {
tokenUrl = INTERNAL_TOKEN_URL // /api/catalog/v1/oauth/tokens
}the login form showing the "Authentication Type" dropdown
Showing the form fields with "Internal (Polaris)" is selected: Client ID, Client Secret, Polaris Realm
Notice the new "Keycloak Realm" field appears
|
|
I am working on this pr and will resolve the conflicts and will remove the exta-spaces. it will be done about in an hour |
|
i have resolved the conflict issues. if i have done anything wrong while resolving conflicts let me know i will fix it later. disclimer: (as i have no experience in resolving the conflict issues on pull request and i asked for chatgpt about selection and then i resolved conflict issues) |
|
Hi @developerzohaib786, thank you for your effort and contribution. I would like to inform you in advance that, as part of the work on another feature request In the new design, Workspace is introduced as a separate entity that contains all the necessary configuration for connecting to an Apache Polaris instance, which will unblock the possibility to use Console UI as an instance for multiple Polaris servers / Realms. A draft PR has already been created and is ready for testing: #138 Due to these upcoming changes, I assume that this PR and the related Issue may become outdated. Even if the changes are accepted, they will most likely be removed, since the server URL and auth endpoint configuration will be moved to the Workspace entity. |
Would you like me to assign some work of upcoming design and logic? |
i think if the changes will be removed or outdated by upcoming changes then let it be as i am preparing for Gsoc and this will be a great favour for me. |
Hello! Thank you for your interest. I can't tell you much what to do next, but any contribution in testing in this PR #138 is very welcome. I know only if you have time and want to help you can join Apache Polaris Community Slack and ask for any advice there: https://join.slack.com/t/apache-polaris/shared_invite/zt-3niqssi92-s2IQMbe1jJ89ayZ4jQv2EQ |



Fixes #98
Make the OAuth token URL configurable via VITE_OAUTH_TOKEN_URL environment variable to support integration with 3rd party providers like Keycloak.
Changes:
Defaults to '/api/catalog/v1/oauth/tokens' for backward compatibility.