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

[AAE-4985] - Make SSO Role Service accept a content admin role that is not part of the JWT token #6942

Merged
merged 9 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/core/services/auth-guard-sso-role.service.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ const appRoutes: Routes = [
```

If the user now clicks on a link or button that follows this route, they will be not able to access this content if they do not have the Realms roles.
<br>**Note**: An additional role ALFRESCO_ADMINISTRATORS can be used in the roles array, which will result in checking whether the logged in user has Content Admin capabilities or not, as this role is not part of the JWT token it will call a Content API to determine it.


Client role Example
Client role Example
```ts
const appRoutes: Routes = [
...
Expand Down
2 changes: 2 additions & 0 deletions e2e/protractor.excludes.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@
"C279931": "login problem APS not basic",
"C279930": "login problem APS not basic",
"C593560": "https://alfresco.atlassian.net/browse/ADF-5366",
"C269081": "https://alfresco.atlassian.net/browse/ADF-5385",
"C272819": "https://alfresco.atlassian.net/browse/ADF-5385",
"C290069": "https://alfresco.atlassian.net/browse/ADF-5387"
}
23 changes: 23 additions & 0 deletions lib/core/mock/ecm-user.service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import { EcmCompanyModel } from '../models/ecm-company.model';
import { PersonEntry, Person } from '@alfresco/js-api';

export let fakeEcmCompany: EcmCompanyModel = {
organization: 'company-fake-name',
Expand Down Expand Up @@ -99,3 +100,25 @@ export const createNewPersonMock = {
password: 'fake-avatar-id',
email: 'fakeEcm@ecmUser.com'
};

export function getFakeUserWithContentAdminCapability(): PersonEntry {
const fakeEcmUserWithAdminCapabilities = {
...fakeEcmUser,
capabilities: {
isAdmin: true
}
};
const mockPerson = new Person(fakeEcmUserWithAdminCapabilities);
return { entry: mockPerson };
}

export function getFakeUserWithContentUserCapability(): PersonEntry {
const fakeEcmUserWithAdminCapabilities = {
...fakeEcmUser,
capabilities: {
isAdmin: false
}
};
const mockPerson = new Person(fakeEcmUserWithAdminCapabilities);
return { entry: mockPerson };
}
40 changes: 40 additions & 0 deletions lib/core/services/auth-guard-sso-role.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,16 @@ import { AuthGuardSsoRoleService } from './auth-guard-sso-role.service';
import { JwtHelperService } from './jwt-helper.service';
import { MatDialog } from '@angular/material/dialog';
import { TranslateModule } from '@ngx-translate/core';
import { PeopleContentService } from './people-content.service';
import { of } from 'rxjs';
import { getFakeUserWithContentAdminCapability, getFakeUserWithContentUserCapability } from '../mock/ecm-user.service.mock';

describe('Auth Guard SSO role service', () => {

let authGuard: AuthGuardSsoRoleService;
let jwtHelperService: JwtHelperService;
let routerService: Router;
let peopleContentService: PeopleContentService;

setupTestBed({
imports: [
Expand All @@ -42,6 +46,7 @@ describe('Auth Guard SSO role service', () => {
authGuard = TestBed.inject(AuthGuardSsoRoleService);
jwtHelperService = TestBed.inject(JwtHelperService);
routerService = TestBed.inject(Router);
peopleContentService = TestBed.inject(PeopleContentService);
});

it('Should canActivate be true if the Role is present int the JWT token', async(async () => {
Expand Down Expand Up @@ -185,4 +190,39 @@ describe('Auth Guard SSO role service', () => {
expect(await authGuard.canActivate(route)).toBeFalsy();
expect(materialDialog.closeAll).toHaveBeenCalled();
});

describe('Content Admin', () => {

afterEach(() => {
peopleContentService.hasCheckedIsContentAdmin = false;
});

it('Should give access to a content section (ALFRESCO_ADMINISTRATORS) when the user has content admin capability', async () => {
spyOn(peopleContentService, 'getCurrentPerson').and.returnValue(of(getFakeUserWithContentAdminCapability()));

const router: ActivatedRouteSnapshot = new ActivatedRouteSnapshot();
router.data = { 'roles': ['ALFRESCO_ADMINISTRATORS'] };

expect(await authGuard.canActivate(router)).toBeTruthy();
});

it('Should not give access to a content section (ALFRESCO_ADMINISTRATORS) when the user does not have content admin capability', async () => {
spyOn(peopleContentService, 'getCurrentPerson').and.returnValue(of(getFakeUserWithContentUserCapability()));

const router: ActivatedRouteSnapshot = new ActivatedRouteSnapshot();
router.data = { 'roles': ['ALFRESCO_ADMINISTRATORS'] };

expect(await authGuard.canActivate(router)).toBeFalsy();
});

it('Should not call the service to check if the user has content admin capability when the roles do not contain ALFRESCO_ADMINISTRATORS', async () => {
const getCurrentPersonSpy = spyOn(peopleContentService, 'getCurrentPerson').and.returnValue(of(getFakeUserWithContentAdminCapability()));
const router: ActivatedRouteSnapshot = new ActivatedRouteSnapshot();
router.data = { 'roles': ['fakeRole'] };

await authGuard.canActivate(router);

expect(getCurrentPersonSpy).not.toHaveBeenCalled();
});
});
});
14 changes: 9 additions & 5 deletions lib/core/services/auth-guard-sso-role.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,28 @@ import { Injectable } from '@angular/core';
import { JwtHelperService } from './jwt-helper.service';
import { ActivatedRouteSnapshot, CanActivate, Router } from '@angular/router';
import { MatDialog } from '@angular/material/dialog';
import { ContentGroups, PeopleContentService } from './people-content.service';

@Injectable({
providedIn: 'root'
})
export class AuthGuardSsoRoleService implements CanActivate {

constructor(private jwtHelperService: JwtHelperService, private router: Router, private dialog: MatDialog) {
constructor(private jwtHelperService: JwtHelperService,
private router: Router,
private dialog: MatDialog,
private peopleContentService: PeopleContentService) {
}

canActivate(route: ActivatedRouteSnapshot): boolean {
async canActivate(route: ActivatedRouteSnapshot): Promise<boolean> {
let hasRole;
let hasRealmRole = false;
let hasClientRole = true;

if (route.data) {
if (route.data['roles']) {
const rolesToCheck = route.data['roles'];
hasRealmRole = this.jwtHelperService.hasRealmRoles(rolesToCheck);
const rolesToCheck: string[] = route.data['roles'];
const isContentAdmin = rolesToCheck.includes(ContentGroups.ALFRESCO_ADMINISTRATORS) ? await this.peopleContentService.isContentAdmin() : false;
eromano marked this conversation as resolved.
Show resolved Hide resolved
hasRealmRole = this.jwtHelperService.hasRealmRoles(rolesToCheck) || isContentAdmin;
}

if (route.data['clientRoles']) {
Expand Down
15 changes: 14 additions & 1 deletion lib/core/services/people-content.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { fakeEcmUser, createNewPersonMock } from '../mock/ecm-user.service.mock';
import { fakeEcmUser, createNewPersonMock, getFakeUserWithContentAdminCapability } from '../mock/ecm-user.service.mock';
import { AlfrescoApiServiceMock } from '../mock/alfresco-api.service.mock';
import { CoreTestingModule } from '../testing/core.testing.module';
import { PeopleContentService } from './people-content.service';
Expand All @@ -24,6 +24,7 @@ import { setupTestBed } from '../testing/setup-test-bed';
import { TranslateModule } from '@ngx-translate/core';
import { TestBed } from '@angular/core/testing';
import { LogService } from './log.service';
import { of } from 'rxjs';

describe('PeopleContentService', () => {

Expand Down Expand Up @@ -101,4 +102,16 @@ describe('PeopleContentService', () => {
done();
});
});

it('Should make the api call to check if the user is a content admin only once', async () => {
const getCurrentPersonSpy = spyOn(service.peopleApi, 'getPerson').and.returnValue(of(getFakeUserWithContentAdminCapability()));

expect(await service.isContentAdmin()).toBe(true);
expect(getCurrentPersonSpy.calls.count()).toEqual(1);

await service.isContentAdmin();

expect(await service.isContentAdmin()).toBe(true);
expect(getCurrentPersonSpy.calls.count()).toEqual(1);
});
});
16 changes: 16 additions & 0 deletions lib/core/services/people-content.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@ import { PersonEntry, PeopleApi, PersonBodyCreate } from '@alfresco/js-api';
import { EcmUserModel } from '../models/ecm-user.model';
import { LogService } from './log.service';

export enum ContentGroups {
ALFRESCO_ADMINISTRATORS = 'ALFRESCO_ADMINISTRATORS'
}

@Injectable({
providedIn: 'root'
})
export class PeopleContentService {
private hasContentAdminRole: boolean = false;
hasCheckedIsContentAdmin: boolean = false;

private _peopleApi: PeopleApi;

Expand Down Expand Up @@ -60,6 +66,7 @@ export class PeopleContentService {
/**
* Creates new person.
* @param newPerson Object containing the new person details.
* @param opts Optional parameters
* @returns Created new person
*/
createPerson(newPerson: PersonBodyCreate, opts?: any): Observable<EcmUserModel> {
Expand All @@ -69,6 +76,15 @@ export class PeopleContentService {
);
}

async isContentAdmin(): Promise<boolean> {
if (!this.hasCheckedIsContentAdmin) {
const user: PersonEntry = await this.getCurrentPerson().toPromise();
this.hasContentAdminRole = user?.entry?.capabilities?.isAdmin;
arditdomi marked this conversation as resolved.
Show resolved Hide resolved
this.hasCheckedIsContentAdmin = true;
}
return this.hasContentAdminRole;
}

private handleError(error: any) {
this.logService.error(error);
return throwError(error || 'Server error');
Expand Down