Skip to content

[NAE-2118] Implement OpenID Connector Auth for Admin node #286

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

Open
wants to merge 6 commits into
base: release/7.0.0-rev3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions nae.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"providers": {
"auth": {
"address": "http://localhost:8080/api/",
"authentication": "Basic",
"authentication": "BasicWithRealm",
"endpoints": {
"login": "auth/login",
"logout": "auth/logout",
Expand All @@ -14,12 +14,19 @@
"verify": "auth/token/verify",
"invite": "auth/invite",
"reset": "auth/reset",
"recover": "/auth/recover"
"recover": "auth/recover"
},
"sessionBearer": "X-Auth-Token",
"sessionTimeoutEnabled": false,
"sessionTimeout": 900,
"jwtBearer": "X-Jwt-Token"
"jwtBearer": "X-Jwt-Token",
"sso": {
"enable": true,
"clientId": "dev-cluster-worker",
"redirectUrl": "https://iam.mudu.dev.netgrif.cloud/realms/netgrif-cloud-testing/protocol/openid-connect/auth",
"refreshUrl": "http://localhost:8800/api/auth/login",
"scopes": ["openid","email","profile","roles"]
}
Comment on lines +23 to +29
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

❓ Verification inconclusive

Mixed-content risk: refreshUrl uses HTTP while the app is likely served over HTTPS

Browsers will block XMLHttpRequests from an HTTPS origin to an HTTP endpoint, breaking the token refresh flow in production.

Please verify that the service is reachable via HTTPS or proxy the call through the same origin.


Mixed-content risk: refreshUrl uses HTTP while the app is likely served over HTTPS

Browsers will block XMLHttpRequests from an HTTPS origin to an HTTP endpoint, breaking the token refresh flow in production.

Please verify that the service is reachable via HTTPS or proxy the call through the same origin.

🤖 Prompt for AI Agents
In nae.json around lines 23 to 29, the refreshUrl is set to use HTTP which can
cause mixed-content issues when the app is served over HTTPS. Update the
refreshUrl to use HTTPS if the service supports it, or configure a proxy on the
same HTTPS origin to forward requests to the HTTP endpoint, ensuring secure and
unblocked token refresh requests.

},
"resources": [
{
Expand Down
1 change: 1 addition & 0 deletions projects/netgrif-components-core/src/assets/i18n/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
"login": "Benutzername",
"wrongCredentials": "Falsche Anmeldeinformationen!",
"loginButton": "Anmelden",
"ssoButton": "SSO-Anmeldung",
"reset": "Kennwort wiederherstellen",
"sign": "Registrieren",
"enterPass": "Ihre Kennwort eingeben"
Expand Down
1 change: 1 addition & 0 deletions projects/netgrif-components-core/src/assets/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
"login": "Username",
"wrongCredentials": "Wrong credentials!",
"loginButton": "Log in",
"ssoButton": "Log with SSO",
"reset": "Reset password",
"sign": "Sign Up",
"enterPass": "Enter your password"
Expand Down
1 change: 1 addition & 0 deletions projects/netgrif-components-core/src/assets/i18n/sk.json
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
"login": "Prihlasovacie meno",
"wrongCredentials": "Nesprávne prihlasovacie údaje!",
"loginButton": "Prihlásiť",
"ssoButton": "Prihlásiť sa pomocou SSO",
"reset": "Obnova hesla",
"sign": "Registrovať",
"enterPass": "Zadajte svoje heslo"
Expand Down
9 changes: 9 additions & 0 deletions projects/netgrif-components-core/src/commons/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,19 @@ export interface Auth {
authentication: string;
sessionBearer?: string;
endpoints?: string | { [k: string]: string };
sso?: Sso;

[k: string]: any;
}

export interface Sso {
enable: boolean;
redirectUrl: string;
refreshUrl: string;
clientId: string;
scopes: Array<string>;
}

export interface Resource {
name: string;
address: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {UserService} from '../../user/services/user.service';
import {User} from '../../user/models/user';
import {LoadingEmitter} from '../../utility/loading-emitter';
import {take} from 'rxjs/operators';
import {ConfigurationService} from "../../configuration/configuration.service";
import {Sso} from "../../../commons/schema";

@Component({
selector: 'ncc-abstract-login-field',
Expand All @@ -15,6 +17,7 @@ export abstract class AbstractLoginFormComponent implements HasForm, OnDestroy {
public rootFormGroup: FormGroup;
public hidePassword = true;
public loading: LoadingEmitter;
protected showSsoButton: boolean;

@Input() public showSignUpButton: boolean;
@Input() public showForgottenPasswordButton: boolean;
Expand All @@ -23,7 +26,7 @@ export abstract class AbstractLoginFormComponent implements HasForm, OnDestroy {
@Output() public signUp: EventEmitter<void>;
@Output() public formSubmit: EventEmitter<FormSubmitEvent>;

protected constructor(formBuilder: FormBuilder, protected _userService: UserService) {
protected constructor(formBuilder: FormBuilder, protected _userService: UserService, protected _config: ConfigurationService) {
this.rootFormGroup = formBuilder.group({
login: [''],
password: ['']
Expand All @@ -33,6 +36,8 @@ export abstract class AbstractLoginFormComponent implements HasForm, OnDestroy {
this.signUp = new EventEmitter<void>();
this.formSubmit = new EventEmitter<FormSubmitEvent>();
this.loading = new LoadingEmitter();
let ssoConfig: Sso = this._config.getConfigurationSubtree(['providers', 'auth', 'sso'])
this.showSsoButton = ssoConfig?.enable
}
Comment on lines +39 to 41
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Defensive defaults & const instead of let

showSsoButton is declared boolean but can currently be undefined.
Also, ssoConfig isn’t reassigned – mark it const.

-        let ssoConfig: Sso = this._config.getConfigurationSubtree(['providers', 'auth', 'sso'])
-        this.showSsoButton = ssoConfig?.enable
+        const ssoConfig: Sso | undefined =
+            this._config.getConfigurationSubtree(['providers', 'auth', 'sso']);
+        this.showSsoButton = !!ssoConfig?.enable;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let ssoConfig: Sso = this._config.getConfigurationSubtree(['providers', 'auth', 'sso'])
this.showSsoButton = ssoConfig?.enable
}
const ssoConfig: Sso | undefined =
this._config.getConfigurationSubtree(['providers', 'auth', 'sso']);
this.showSsoButton = !!ssoConfig?.enable;
}
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/forms/login/abstract-login-form.component.ts
around lines 39 to 41, change the declaration of ssoConfig from let to const
since it is not reassigned. Also, ensure showSsoButton is assigned a boolean
value by providing a defensive default (e.g., false) when ssoConfig or
ssoConfig.enable is undefined, so showSsoButton never becomes undefined.


ngOnDestroy(): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// todo 2118
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import {Component, OnDestroy} from "@angular/core";
import {ActivatedRoute, Params, Router} from "@angular/router";
import {Observable, throwError} from "rxjs";
import {catchError} from "rxjs/operators";
import {HttpClient} from "@angular/common/http";
import {ConfigurationService} from "../../../configuration/configuration.service";
import {LoggerService} from '../../../logger/services/logger.service';
import {LoadingEmitter} from '../../../utility/loading-emitter';
import {SnackBarService} from "../../../snack-bar/services/snack-bar.service";
import {Sso} from "../../../../commons/schema";
import {TranslateService} from "@ngx-translate/core";


@Component({
selector: 'ncc-abstract-login-field',
template: ''
})
export abstract class AbstractLoginSsoComponent implements OnDestroy {

private _ssoConfig: Sso;
protected loading: LoadingEmitter;

protected constructor(
protected _config: ConfigurationService,
protected _http: HttpClient,
protected _snackbar: SnackBarService,
protected _log: LoggerService,
protected _router: Router,
protected _activeRouter: ActivatedRoute,
protected _translate: TranslateService
) {
this._ssoConfig = this._config.getConfigurationSubtree(['providers', 'auth', 'sso']);
this.loading = new LoadingEmitter();
Comment on lines +20 to +33
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Guard against missing SSO configuration
_config.getConfigurationSubtree() can legally return undefined if the subtree is not present. Directly dereferencing the resulting object (this._ssoConfig.clientId, this._ssoConfig.scopes, …) will then throw at run time.

- this._ssoConfig = this._config.getConfigurationSubtree(['providers', 'auth', 'sso']);
+ const subtree = this._config.getConfigurationSubtree(['providers', 'auth', 'sso']);
+ if (!subtree) {
+     this._log.error('SSO configuration subtree is missing – SSO button will not work');
+     return;
+ }
+ this._ssoConfig = subtree;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/forms/login/login-sso/abstract-login-sso.component.ts
around lines 20 to 33, the assignment of this._ssoConfig directly from
_config.getConfigurationSubtree() can result in undefined if the configuration
subtree is missing. To fix this, add a guard to check if the returned value is
undefined and handle that case appropriately, such as initializing
this._ssoConfig with a default object or adding null checks before accessing its
properties to prevent runtime errors.

this._activeRouter.queryParams.subscribe((params) => {
if (!!params.code) {
this.loginFromCode(params);
}
});
}
Comment on lines +34 to +39
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Subscription is never disposed – memory-leak risk
The component subscribes to queryParams in the constructor but does not retain nor unsubscribe from the resulting Subscription. When the component is destroyed, the stream will keep the component instance alive.

+import {Subscription} from 'rxjs';- this._activeRouter.queryParams.subscribe((params) => {
+ this._paramsSub = this._activeRouter.queryParams.subscribe(params => {
     if (params.code) {
         this.loginFromCode(params);
     }
 });

and in the class body / ngOnDestroy:

- ngOnDestroy(): void {
-     this.loading.complete();
- }
+ private _paramsSub?: Subscription;
+
+ ngOnDestroy(): void {
+     this._paramsSub?.unsubscribe();
+     this.loading.complete();
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this._activeRouter.queryParams.subscribe((params) => {
if (!!params.code) {
this.loginFromCode(params);
}
});
}
// at the top of the file
import { Subscription } from 'rxjs';
export class AbstractLoginSsoComponent implements OnDestroy {
// add a field to hold the subscription
private _paramsSub?: Subscription;
constructor(
private _activeRouter: ActivatedRoute,
// …
) {
// capture the subscription so we can unsubscribe later
this._paramsSub = this._activeRouter.queryParams.subscribe(params => {
if (params.code) {
this.loginFromCode(params);
}
});
}
// …
ngOnDestroy(): void {
// tear down the router subscription
this._paramsSub?.unsubscribe();
this.loading.complete();
}
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 35-35: Avoid redundant double-negation.

It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation

(lint/complexity/noExtraBooleanCast)

🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/forms/login/login-sso/abstract-login-sso.component.ts
around lines 34 to 39, the subscription to this._activeRouter.queryParams is
created but never stored or unsubscribed, causing a potential memory leak. Fix
this by storing the subscription in a class property and then unsubscribing from
it in the ngOnDestroy lifecycle method to properly clean up when the component
is destroyed.


ngOnDestroy(): void {
this.loading.complete();
}

public redirectToSso(): void {
let redirectUrl: string = this.getRedirectUrl();
this._log.info("Redirecting to " + redirectUrl)
window.location.href = redirectUrl;
}

public loginFromCode(params: Params) {
if (!params.code) {
return;
}

this.loading.on();
this._log.debug('Handling access token: ' + params.code)
const token$ = this.getToken({
Comment on lines +57 to +58
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Authorization code is sensitive – do not log it
this._log.debug('Handling access token: ' + params.code) exposes the OAuth 2.0 authorization-code in logs, creating an unnecessary attack surface. Remove the value or mask it.

- this._log.debug('Handling access token: ' + params.code)
+ this._log.debug('Handling authorization-code callback')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this._log.debug('Handling access token: ' + params.code)
const token$ = this.getToken({
this._log.debug('Handling authorization-code callback')
const token$ = this.getToken({
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/forms/login/login-sso/abstract-login-sso.component.ts
at lines 57-58, the code logs the OAuth 2.0 authorization code in plain text,
which is sensitive information. Modify the debug log to either remove the
authorization code entirely or replace it with a masked or generic placeholder
to avoid exposing sensitive data in logs.

grantType: 'authorization_code',
code: params.code,
realmId: '', // todo send realm id
redirectUri: location.origin + '/' + this._config.getConfigurationSubtree(['services', 'auth', 'toLoginRedirect']),
});
token$.subscribe(
token => {
this.loading.off();
if (!!token) {
this.redirectToHome();
}
},
);
}

private getRedirectUrl(): string {
const myQuery = this._ssoConfig.redirectUrl + '?';
const options: { [index: string]: string } = {
client_id: this._ssoConfig.clientId,
redirect_uri: location.origin + '/' + this._config.getConfigurationSubtree(['services', 'auth', 'toLoginRedirect']),
response_type: 'code',
scope: this._ssoConfig.scopes.join(' '),
};
return myQuery + Object.keys(options).map(
key => {
return encodeURIComponent(key) + '=' + encodeURIComponent(options[key]);
},
).join('&');
}

private getToken(body: any): Observable<any> {
const url = this._ssoConfig.refreshUrl;
if (!url) {
return throwError(() => new Error('Refresh URL is not defined in the config [nae.providers.auth.sso.refreshUrl]'));
}
return this._http.post(url, body,
{headers: {'Content-Type': 'application/json'}}).pipe(
catchError(error => {
this.loading.off();
this._snackbar.openErrorSnackBar(this._translate.instant('forms.login.wrongCredentials'));
return throwError(() => error);
}),
);
}

private redirectToHome() {
this._router.navigate(['/' + this._config.getConfigurationSubtree(['services', 'auth', 'onLoginRedirect'])])
.then((value) => { this._log.debug('Routed to ' + value); });
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './email-submission/abstract-email-submission-form.component';
export * from './login/abstract-login-form.component';
export * from './login/login-sso/abstract-login-sso.component';
export * from './registration/abstract-registration-form.component';
export * from './forgotten-password/abstract-forgotten-password.component';
export * from './models/abstract-registration.component';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
<span *ngIf="(loading | async) === false" fxFlex>{{ 'forms.login.loginButton' | translate}}</span>
</button>
</div>
<div fxLayout="row" [fxLayoutAlign]="getButtonsFxLayoutAlign()" fxLayoutAlign.xs="center" class="margin-top-4">
<nc-login-sso *ngIf="showSsoButton"></nc-login-sso>
</div>
</form>

<div *ngIf="showSignUpButton && showForgottenPasswordButton" fxLayout="row" class="width-100" fxLayoutAlign="end">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {Component} from '@angular/core';
import {FormBuilder} from '@angular/forms';
import {AbstractLoginFormComponent, UserService} from '@netgrif/components-core';
import {AbstractLoginFormComponent, ConfigurationService, UserService} from '@netgrif/components-core';

@Component({
selector: 'nc-login-form',
templateUrl: './login-form.component.html',
styleUrls: ['./login-form.component.scss']
})
export class LoginFormComponent extends AbstractLoginFormComponent {
constructor(formBuilder: FormBuilder, protected _userService: UserService) {
super(formBuilder, _userService);
constructor(formBuilder: FormBuilder, protected _userService: UserService, protected _config: ConfigurationService) {
super(formBuilder, _userService, _config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import {FlexLayoutModule} from '@ngbracket/ngx-layout';
import {ReactiveFormsModule} from '@angular/forms';
import {LoginFormComponent} from './login-form.component';
import {MaterialModule, TranslateLibModule} from '@netgrif/components-core';
import {LoginSsoComponent} from "./login-sso/login-sso.component";


@NgModule({
declarations: [LoginFormComponent],
exports: [LoginFormComponent],
declarations: [LoginFormComponent, LoginSsoComponent],
exports: [LoginFormComponent, LoginSsoComponent],
imports: [
CommonModule,
MaterialModule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<button color="primary" (click)="redirectToSso()" [disabled]="(loading | async)" mat-raised-button fxLayout="row"
fxLayoutAlign="center center">
<mat-spinner *ngIf="loading | async"
mode="indeterminate"
diameter="36"
color="accent"
fxFlex></mat-spinner>
<span *ngIf="(loading | async) === false" fxFlex>{{ 'forms.login.ssoButton' | translate }}</span>
</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';

import { LoginSsoComponent } from './login-sso.component';

// todo 2118

xdescribe('LoginSsoComponent', () => {
let component: LoginSsoComponent;
let fixture: ComponentFixture<LoginSsoComponent>;

beforeEach(async () => {
await TestBed.configureTestingModule({
declarations: [ LoginSsoComponent ]
})
.compileComponents();
});

beforeEach(() => {
fixture = TestBed.createComponent(LoginSsoComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

it('should create', () => {
expect(component).toBeTruthy();
});
});
Comment on lines +5 to +27
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Disabled test suite hides compilation/runtime issues

The whole spec is wrapped in xdescribe, so it never runs.

  1. Potential template-compilation errors inside LoginSsoComponent will stay undetected.
  2. Test coverage tools will ignore the component entirely.

Unless there is a strong, short-term reason to skip, turn it into an active suite (or delete it until ready).

-xdescribe('LoginSsoComponent', () => {
+describe('LoginSsoComponent', () => {

If the component depends on external modules (Material, TranslateLib, etc.) add them to the testing module to prevent runtime failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// todo 2118
xdescribe('LoginSsoComponent', () => {
let component: LoginSsoComponent;
let fixture: ComponentFixture<LoginSsoComponent>;
beforeEach(async () => {
await TestBed.configureTestingModule({
declarations: [ LoginSsoComponent ]
})
.compileComponents();
});
beforeEach(() => {
fixture = TestBed.createComponent(LoginSsoComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});
it('should create', () => {
expect(component).toBeTruthy();
});
});
// todo 2118
describe('LoginSsoComponent', () => {
let component: LoginSsoComponent;
let fixture: ComponentFixture<LoginSsoComponent>;
beforeEach(async () => {
await TestBed.configureTestingModule({
declarations: [ LoginSsoComponent ]
})
.compileComponents();
});
beforeEach(() => {
fixture = TestBed.createComponent(LoginSsoComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});
it('should create', () => {
expect(component).toBeTruthy();
});
});
🤖 Prompt for AI Agents
In
projects/netgrif-components/src/lib/forms/login/login-sso/login-sso.component.spec.ts
between lines 5 and 27, the test suite is disabled using xdescribe, which
prevents the tests from running and hides potential compilation or runtime
errors. To fix this, remove the 'x' prefix to activate the describe block so the
tests execute. Additionally, ensure that any external modules required by
LoginSsoComponent, such as Material or TranslateLib, are imported and included
in the TestBed configuration to avoid runtime failures during testing.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import {HttpClient} from '@angular/common/http';
import {Component} from '@angular/core';
import {ActivatedRoute, Router} from '@angular/router';
import {AbstractLoginSsoComponent, ConfigurationService, LoggerService, SnackBarService} from '@netgrif/components-core';
import {TranslateService} from "@ngx-translate/core";

@Component({
selector: 'nc-login-sso',
templateUrl: './login-sso.component.html',
styleUrls: ['./login-sso.component.scss'],
})
export class LoginSsoComponent extends AbstractLoginSsoComponent {
constructor(
protected _config: ConfigurationService,
protected _http: HttpClient,
protected _snackbar: SnackBarService,
protected _log: LoggerService,
protected _router: Router,
protected _activeRouter: ActivatedRoute,
protected _translate: TranslateService
) {
super(_config, _http, _snackbar, _log, _router, _activeRouter, _translate);
}
}
1 change: 1 addition & 0 deletions projects/netgrif-components/src/lib/forms/public-api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* COMPONENTS */
export * from './email-submission/email-submission-form.component';
export * from './login/login-form.component';
export * from './login/login-sso/login-sso.component';
export * from './registration/registration-form.component';
export * from './forgotten-password/forgotten-password-form.component';
export * from './change-password/change-password-form.component';
Expand Down
Loading