Skip to content

PoC SSO - Keycloak integration#3105

Merged
GermanBluefox merged 28 commits intomasterfrom
keycloak
Jun 17, 2025
Merged

PoC SSO - Keycloak integration#3105
GermanBluefox merged 28 commits intomasterfrom
keycloak

Conversation

@foxriver76
Copy link
Copy Markdown
Collaborator

@foxriver76 foxriver76 commented May 14, 2025

commonjs(),
],
server: {
host: '0.0.0.0',
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This allows dev-server to listen also to 127.0.0.1 instead of only localhost else local testing with SSO is not possible.

@@ -1,3 +1,4 @@
/// <reference types="vite/client" />
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Gives us types for import.meta.env.DEV etc

Comment thread package.json
"@iobroker/types": "^7.0.7",
"@alcalzone/release-script-plugin-license": "^3.7.0",
"@iobroker/dm-utils": "^1.0.10",
"@iobroker/eslint-config": "^2.0.1",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

moved eslint config to the package root (was only present in adapter-react-v5), else the backend had no ESLint support at least for my setup.

@GermanBluefox GermanBluefox requested a review from Copilot May 19, 2025 06:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a proof-of-concept SSO integration with Keycloak, adding proxy support, UI controls, and callback handling.

  • Exposes a /sso proxy in the dev server and adds an SSO button on the login page
  • Implements OIDC connect/disconnect buttons in the user edit dialog and updates related type signatures
  • Handles SSO callback parameters in the main App component and adds translation keys for Single-Sign On

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/admin/src-admin/vite.config.ts Enabled host 0.0.0.0 and added /sso proxy
packages/admin/src-admin/src/login/Login.tsx Adjusted avatar rendering logic and added SSO login button
packages/admin/src-admin/src/i18n/* Added “Single-Sign On” and “Disconnect Single-Sign On” keys
packages/admin/src-admin/src/components/Users/UserEditDialog.tsx Added OIDC connect/disconnect buttons; updated saveData to return a promise
packages/admin/src-admin/src/App.tsx Added componentDidUpdate to process SSO callback params
packages/adapter-react-v5/package.json Removed unused @iobroker/eslint-config dev dependency
package.json Reordered and added @iobroker/adapter-react-v5 dependency

Comment on lines +281 to +287
window.loginHideLogo === 'false' ||
(window.loginHideLogo === '@@loginHideLogo@@' && (
<Avatar
sx={styles.avatar}
src="img/admin.svg"
/>
)
))
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

Using logical OR here can render true in the JSX when loginHideLogo is 'false'. Replace with (cond1 || cond2) && <Avatar> so only the <Avatar> is rendered when either condition is met.

Copilot uses AI. Check for mistakes.
"Discard": "Discard",
"Discard unsaved changes?": "Discard unsaved changes?",
"Discard?": "Discard?",
"Disconnect Single-Sign On'": "Disconnect Single-Sign On'",
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

Translation key has an extra trailing apostrophe. Remove the apostrophe so it matches your lookup string Disconnect Single-Sign On.

Suggested change
"Disconnect Single-Sign On'": "Disconnect Single-Sign On'",
"Disconnect Single-Sign On": "Disconnect Single-Sign On",

Copilot uses AI. Check for mistakes.
*/
renderOidcButton(): JSX.Element {
/* @ts-expect-error needs to be added to types */
return this.props.user.common?.externalAuthentication?.oidc ? (
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

The branches appear inverted: when oidc exists you render 'Add Single-Sign On', but it should likely render the disconnect button. Swap the branches or correct the labels.

Copilot uses AI. Check for mistakes.
Comment thread packages/admin/src-admin/src/components/Users/UserEditDialog.tsx
Comment thread packages/admin/src-admin/src/components/Users/UserEditDialog.tsx
Comment thread packages/admin/src-admin/src/App.tsx Outdated
Comment on lines +736 to +738
componentDidUpdate(): void {
// Due to the fact that the SSO process can only provide is parameters via a callback uri, we need to extract from the search parameters
// However, there might be a better place for this instead of using a side effect on every re-render
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Parsing URL search parameters on every update can be inefficient. Consider moving this logic to componentDidMount or using a one-time effect.

Suggested change
componentDidUpdate(): void {
// Due to the fact that the SSO process can only provide is parameters via a callback uri, we need to extract from the search parameters
// However, there might be a better place for this instead of using a side effect on every re-render
componentDidMount(): void {
// Due to the fact that the SSO process can only provide its parameters via a callback URI, we need to extract them from the search parameters

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@foxriver76 Why not in componentDidMount ?

Comment thread packages/admin/src-admin/public/lib/js/ace/ext-searchbox.js Fixed
- can only be turned on by editing the JSON right now as it is only for testing
@foxriver76 foxriver76 marked this pull request as ready for review June 16, 2025 08:56
@foxriver76 foxriver76 requested a review from GermanBluefox June 16, 2025 08:56
Copy link
Copy Markdown
Contributor

@GermanBluefox GermanBluefox left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread packages/admin/src-admin/src/App.tsx Outdated
Comment on lines +736 to +738
componentDidUpdate(): void {
// Due to the fact that the SSO process can only provide is parameters via a callback uri, we need to extract from the search parameters
// However, there might be a better place for this instead of using a side effect on every re-render
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@foxriver76 Why not in componentDidMount ?

Comment thread packages/admin/src-admin/src/App.tsx Outdated
}

componentDidUpdate(): void {
// Due to the fact that the SSO process can only provide is parameters via a callback uri, we need to extract from the search parameters
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is parameters => its parameters ?

@GermanBluefox GermanBluefox merged commit 2499d8b into master Jun 17, 2025
18 checks passed
@GermanBluefox GermanBluefox deleted the keycloak branch June 17, 2025 11:13
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