Skip to content
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
38 changes: 23 additions & 15 deletions src/app/agents/new-agent/new-agent.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,23 @@
<app-page-title title="New Agent" [subbutton]="false"></app-page-title>
<mat-stepper #stepper>
<mat-step label="Download Agent Binary" state="download">
<app-agent-binaries-table

[isSelectable]="false"
></app-agent-binaries-table>
@if (canReadAgentBinaries) {
<app-agent-binaries-table
[isSelectable]="false"
></app-agent-binaries-table>
}
<div class="highlight-box">
<p>
<strong>Download the agent binary</strong> and execute it on the
client server. You can download an agent binary by clicking on the
action menu (...) in the agent binaries table above.
</p>
@if (canReadAgentBinaries) {
<p>
<strong>Download the agent binary</strong> and execute it on the
client server. You can download an agent binary by clicking on the
action menu (...) in the agent binaries table above.
</p>
} @else {
<p>
You do not have permission to view agent binaries. You can skip this step and proceed to register the agent.
</p>
}
</div>
<div>
<button-submit mat-button (click)="stepper.next()" name="Next"></button-submit>
Expand Down Expand Up @@ -53,16 +60,17 @@
</form>
<app-vouchers-table #table></app-vouchers-table>
<div class="highlight-box">
<p *ngIf="!allowMultiVoucher; else multipleVouchersMessage">
<strong>Generate a voucher</strong> to register the agent. Note that once the voucher
is used it will be automatically deleted.
</p>
<ng-template #multipleVouchersMessage>
@if (!allowMultiVoucher) {
<p>
<strong>Generate a voucher</strong> to register the agent. Note that once the voucher
is used it will be automatically deleted.
</p>
} @else {
<p>
<strong>Generate vouchers</strong> to register agents. Vouchers remain available when the
"Register Multiple Agents Using Voucher(s)" option is enabled.
</p>
</ng-template>
}
</div>
<div>
<button-submit mat-button (click)="stepper.previous()" name="Back" type="delete"></button-submit>
Expand Down
47 changes: 46 additions & 1 deletion src/app/agents/new-agent/new-agent.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Perm } from '@constants/userpermissions.config';
import { of } from 'rxjs';

import { Clipboard } from '@angular/cdk/clipboard';
Expand All @@ -13,6 +14,7 @@ import { Router } from '@angular/router';

import { SERV } from '@services/main.config';
import { GlobalService } from '@services/main.service';
import { PermissionService } from '@services/permission/permission.service';
import { AlertService } from '@services/shared/alert.service';
import { ConfigService } from '@services/shared/config.service';

Expand All @@ -28,7 +30,7 @@ import { TableModule } from '@src/app/shared/table/table-actions.module';
standalone: false
})
export class MockAgentBinariesTableComponent {
@Input() isSelectable: boolean;
@Input() isSelectable: boolean = false;
}

// Voucher table mock
Expand All @@ -48,13 +50,16 @@ describe('NewAgentComponent', () => {
let alertServiceSpy: jasmine.SpyObj<AlertService>;
let configServiceSpy: jasmine.SpyObj<ConfigService>;
let globalServiceSpy: jasmine.SpyObj<GlobalService>;
let permissionServiceSpy: jasmine.SpyObj<PermissionService>;
let routerSpy: jasmine.SpyObj<Router>;

beforeEach(async () => {
clipboardSpy = jasmine.createSpyObj('Clipboard', ['copy']);
alertServiceSpy = jasmine.createSpyObj('AlertService', ['showSuccessMessage']);
configServiceSpy = jasmine.createSpyObj('ConfigService', ['getEndpoint']);
globalServiceSpy = jasmine.createSpyObj('GlobalService', ['create', 'getAll']);
permissionServiceSpy = jasmine.createSpyObj('PermissionService', ['hasPermissionSync']);
permissionServiceSpy.hasPermissionSync.and.returnValue(true);
routerSpy = jasmine.createSpyObj('Router', ['navigate']);

// Provide default stub for configService.getEndpoint()
Expand All @@ -80,6 +85,7 @@ describe('NewAgentComponent', () => {
{ provide: AlertService, useValue: alertServiceSpy },
{ provide: ConfigService, useValue: configServiceSpy },
{ provide: GlobalService, useValue: globalServiceSpy },
{ provide: PermissionService, useValue: permissionServiceSpy },
{ provide: Router, useValue: routerSpy }
]
}).compileComponents();
Expand Down Expand Up @@ -175,4 +181,43 @@ describe('NewAgentComponent', () => {
component.ngOnDestroy();
expect(component.newVoucherSubscription.unsubscribe).toHaveBeenCalled();
});

it('should allow agent creation for a user without AccessGroup.READ permission', fakeAsync(() => {
// Simulate a user who lacks AccessGroup.READ but retains Agent.CREATE, Agent.READ and Voucher.READ
// This is the exact scenario described in issue #1955
permissionServiceSpy.hasPermissionSync.and.callFake((perm: string) => perm !== Perm.GroupAccess.READ);

fixture = TestBed.createComponent(NewAgentComponent);
component = fixture.componentInstance;
fixture.detectChanges();

// Component initializes without errors
expect(component).toBeTruthy();

// User can still create a voucher — the actual agent registration action
const voucher = 'fy7vjq56';
component.form.controls['voucher'].setValue(voucher);
component.table = jasmine.createSpyObj('VouchersTableComponent', ['reload']);
globalServiceSpy.create.and.returnValue(of({}));

component.onSubmit();
tick();

expect(globalServiceSpy.create).toHaveBeenCalledWith(SERV.VOUCHER, { voucher: voucher });
expect(alertServiceSpy.showSuccessMessage).toHaveBeenCalledWith('New voucher successfully created!');
}));

it('should not make any access group API call during agent creation', () => {
// Agent creation does not require or fetch access groups — verifies no extraneous dependency
const calledEndpoints = globalServiceSpy.getAll.calls.allArgs().map((args: unknown[]) => args[0]);
expect(calledEndpoints).not.toContain(SERV.ACCESS_GROUPS);
});

it('should hide agent binaries table and avoid 403 when user lacks AgentBinary.READ permission', () => {
// Secondary fix: users without AgentBinary.READ won't trigger a 403 on the binaries endpoint
component.canReadAgentBinaries = false;
fixture.detectChanges();
const table = fixture.nativeElement.querySelector('app-agent-binaries-table');
expect(table).toBeNull();
});
});
4 changes: 4 additions & 0 deletions src/app/agents/new-agent/new-agent.component.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Perm } from '@constants/userpermissions.config';
import { Subscription } from 'rxjs';
import { VouchersTableComponent } from 'src/app/core/_components/tables/vouchers-table/vouchers-table.component';
import { GlobalService } from 'src/app/core/_services/main.service';
Expand All @@ -10,6 +11,7 @@ import { FormControl, FormGroup } from '@angular/forms';
import { Router } from '@angular/router';

import { SERV } from '@services/main.config';
import { PermissionService } from '@services/permission/permission.service';
import { AlertService } from '@services/shared/alert.service';

import { VoucherForm } from '@src/app/agents/new-agent/new-agent.form';
Expand All @@ -30,6 +32,7 @@ export class NewAgentComponent implements OnInit, OnDestroy {
private alertService = inject(AlertService);
private cs = inject(ConfigService);
private gs = inject(GlobalService);
private permissionService = inject(PermissionService);
private router = inject(Router);

form: FormGroup<VoucherForm> = new FormGroup<VoucherForm>({
Expand All @@ -38,6 +41,7 @@ export class NewAgentComponent implements OnInit, OnDestroy {
agentURL: string;
newVoucherSubscription: Subscription;
allowMultiVoucher = false;
canReadAgentBinaries = this.permissionService.hasPermissionSync(Perm.AgentBinary.READ);

@ViewChild('table') table: VouchersTableComponent;

Expand Down
9 changes: 7 additions & 2 deletions src/app/core/_services/main.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,14 @@ export class GlobalService {
.pipe(debounceTime(2000));
}

getRelationships(serviceConfig: ServiceConfig, id: number, relType: string): Observable<any> {
getRelationships(
serviceConfig: ServiceConfig,
id: number,
relType: string,
options?: { headers?: HttpHeaders }
): Observable<any> {
return this.http
.get<ResponseWrapper>(this.cs.getEndpoint() + serviceConfig.URL + '/' + id + '/' + relType)
.get<ResponseWrapper>(this.cs.getEndpoint() + serviceConfig.URL + '/' + id + '/' + relType, options)
.pipe(debounceTime(2000));
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/core/_services/roles/agents/agent-role.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class AgentRoleService extends RoleService {
readChunk: [Perm.Task.READ, Perm.TaskWrapper.READ, Perm.Chunk.READ],
readAccessGroup: [Perm.GroupAccess.READ],
readError: [Perm.AgentError.READ],
create: [Perm.Agent.CREATE, Perm.Agent.READ, Perm.Voucher.READ, Perm.AgentBinary.READ],
create: [Perm.Agent.CREATE, Perm.Agent.READ, Perm.Voucher.READ],
update: [Perm.Agent.UPDATE],
updateAssignment: [Perm.AgentAssignment.UPDATE, Perm.AgentAssignment.READ, Perm.Task.READ, Perm.TaskWrapper.READ]
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { RoleService } from '@services/roles/base/role.service';
export class HashListRoleService extends RoleService {
constructor(permissionService: PermissionService) {
super(permissionService, {
create: [Perm.Hashlist.CREATE, Perm.Hashtype.READ, Perm.GroupAccess.READ],
create: [Perm.Hashlist.CREATE, Perm.Hashtype.READ],
read: [Perm.Hashlist.READ],
update: [Perm.Hashlist.UPDATE],
tasks: [Perm.TaskWrapper.READ],
Expand Down
52 changes: 48 additions & 4 deletions src/app/files/new-files/new-files.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { of } from 'rxjs';
import { of, throwError } from 'rxjs';

import { HttpHeaders } from '@angular/common/http';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { ReactiveFormsModule } from '@angular/forms';
import { MatIconModule } from '@angular/material/icon';
Expand Down Expand Up @@ -205,17 +206,60 @@ describe('NewFilesComponent', () => {
});

describe('Access group scoping', () => {
it('should fetch access groups via getRelationships for the current user, not getAll', () => {
it('should fetch access groups via getRelationships with X-Skip-Error-Dialog header', () => {
setup('wordlist-new');
const gs = TestBed.inject(GlobalService) as unknown as MockGlobalService;

// Component must use getRelationships to get user-scoped access groups
expect(gs.getRelationships).toHaveBeenCalledWith(SERV.USERS, 1, RelationshipType.ACCESSGROUPS);
const callArgs = gs.getRelationships.calls.mostRecent().args;
expect(callArgs[0]).toEqual(SERV.USERS);
expect(callArgs[1]).toBe(1);
expect(callArgs[2]).toBe(RelationshipType.ACCESSGROUPS);
expect(callArgs[3]).toBeDefined();
expect(callArgs[3].headers).toBeInstanceOf(HttpHeaders);
expect(callArgs[3].headers.get('X-Skip-Error-Dialog')).toBe('true');

// getAll must NOT be called — the component should not fetch all access groups
expect(gs.getAll).not.toHaveBeenCalled();
});

it('should fall back to default access group when getRelationships returns error (403)', async () => {
TestBed.overrideProvider(GlobalService, {
useValue: {
...new MockGlobalService(),
getRelationships: jasmine
.createSpy('getRelationships')
.and.returnValue(throwError(() => new Error('403 Forbidden'))),
userId: 1
}
});
TestBed.overrideProvider(ActivatedRoute, {
useValue: { data: of({ kind: 'wordlist-new' }) }
});

fixture = TestBed.createComponent(NewFilesComponent);
component = fixture.componentInstance;
fixture.detectChanges();
await fixture.whenStable();

expect(component.selectAccessgroup.length).toBe(1);
expect(component.selectAccessgroup[0]).toEqual({ id: '1', name: 'Default' });
expect(component.form.get('accessGroupId').value).toBe(1);
expect(component.form.get('accessGroupId').disabled).toBeTrue();
expect(component.isLoading).toBeFalse();
});

it('should fall back to default access group when response has empty data', async () => {
// Default mock already returns { data: [], included: [] } → empty → triggers fallback
setup('wordlist-new');
await fixture.whenStable();

expect(component.selectAccessgroup.length).toBe(1);
expect(component.selectAccessgroup[0]).toEqual({ id: '1', name: 'Default' });
expect(component.form.get('accessGroupId').value).toBe(1);
expect(component.form.get('accessGroupId').disabled).toBeTrue();
expect(component.isLoading).toBeFalse();
});

it('should correctly transform access group API data to select options', () => {
// Simulate deserialized access groups (what JsonAPISerializer would produce)
const deserialized = [
Expand Down
Loading
Loading