Skip to content

Commit

Permalink
fix!: approved threat models should be read-only (again).
Browse files Browse the repository at this point in the history
DefaultAuthzProvider (and inheriting classes) now requires the DataAccessLayer, which will break some configuration.

Fixes #58
  • Loading branch information
Tethik committed Oct 18, 2023
1 parent 794badf commit 34d9da3
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 14 deletions.
2 changes: 1 addition & 1 deletion api/src/test-util/testConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const testConfig: GramConfiguration = {
componentClasses: classes.map(toComponentClass),
identityProviders: [new DummyIdentityProvider()],
reviewerProvider: testReviewerProvider,
authzProvider: new DefaultAuthzProvider(),
authzProvider: new DefaultAuthzProvider(dal),
userProvider: testUserProvider,
systemProvider: testSystemProvider,
suggestionSources: [],
Expand Down
1 change: 1 addition & 0 deletions config/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export const defaultConfig: GramConfiguration = {
fallbackReviewer
),
authzProvider: new StaticAuthzProvider(
dal,
[sampleUsers[0].sub],
[sampleUsers[1].sub],
[sampleUsers[2].sub]
Expand Down
4 changes: 3 additions & 1 deletion config/providers/static/StaticAuthzProvider.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { DefaultAuthzProvider } from "@gram/core/dist/auth/DefaultAuthzProvider.js";
import { Role } from "@gram/core/dist/auth/models/Role.js";
import { DataAccessLayer } from "@gram/core/dist/data/dal.js";

export class StaticAuthzProvider extends DefaultAuthzProvider {
key: string = "static";
constructor(
dal: DataAccessLayer,
public users: string[],
public reviewers: string[],
public admins: string[]
) {
super();
super(dal);
}

async getRolesForUser(sub: string): Promise<Role[]> {
Expand Down
42 changes: 33 additions & 9 deletions core/src/auth/DefaultAuthzProvider.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { DataAccessLayer } from "../data/dal.js";
import Model from "../data/models/Model.js";
import { RequestContext } from "../data/providers/RequestContext.js";
import { ReviewStatus } from "../data/reviews/Review.js";
import { systemProvider } from "../data/systems/systems.js";
import { AuthzProvider } from "./AuthzProvider.js";
import { AllPermissions, Permission } from "./authorization.js";
import { Role } from "./models/Role.js";
import { UserToken } from "./models/UserToken.js";

export class DefaultAuthzProvider implements AuthzProvider {
constructor(protected dal: DataAccessLayer) {}

async getPermissionsForSystem(
ctx: RequestContext,
systemId: string,
Expand Down Expand Up @@ -54,34 +58,54 @@ export class DefaultAuthzProvider implements AuthzProvider {
model: Model,
user: UserToken
): Promise<Permission[]> {
const review = await this.dal.reviewService.getByModelId(model.id!);
if (model.systemId) {
return this.getPermissionsForSystem(ctx, model.systemId, user);
let systemPermissions = await this.getPermissionsForSystem(
ctx,
model.systemId,
user
);
if (review?.status === ReviewStatus.Approved) {
systemPermissions = systemPermissions.filter(
(p) => p !== Permission.Write
);
}
return systemPermissions;
}

if (user.roles.length === 0) return [];

if (user.roles.find((r) => r === Role.Admin)) return AllPermissions;
const permissions: Set<Permission> = new Set();

if (user.roles.find((r) => r === Role.Admin)) {
AllPermissions.forEach((p) => permissions.add(p));
}

/**
* Standalone models are mainly used for training. To avoid authz issues here we allow most things
* by most users. Ideally here there should be some sharing system.
*/

const permissions: Permission[] = [];

if (user.roles.find((r) => r === Role.Reviewer)) {
permissions.push(Permission.Read, Permission.Review, Permission.Write);
[Permission.Read, Permission.Review, Permission.Write].forEach((p) =>
permissions.add(p)
);
}

if (user.roles.find((r) => r === Role.User)) {
permissions.push(Permission.Read, Permission.Write);
[Permission.Read, Permission.Write].forEach((p) => permissions.add(p));
}

if (model.createdBy === user.sub) {
permissions.push(Permission.Read, Permission.Write, Permission.Delete);
[Permission.Read, Permission.Write, Permission.Delete].forEach((p) =>
permissions.add(p)
);
}

return permissions;
if (review?.status === ReviewStatus.Approved) {
permissions.delete(Permission.Write);
}

return [...permissions];
}

async getRolesForUser(sub: string): Promise<Role[]> {
Expand Down
1 change: 0 additions & 1 deletion core/src/auth/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { RequestContext } from "../data/providers/RequestContext.js";
import { NotFoundError } from "../util/errors.js";
import { AuthzError } from "./AuthzError.js";
import { AuthzProvider } from "./AuthzProvider.js";
import { DefaultAuthzProvider } from "./DefaultAuthzProvider.js";
import { DummyAuthzProvider } from "./DummyAuthzProvider.js";
import { Role } from "./models/Role.js";
import { UserToken } from "./models/UserToken.js";
Expand Down
1 change: 1 addition & 0 deletions plugins/ldap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Plugin for different providers using LDAP. Warning: these may require some extra
- **LDAPGroupBasedAuthzProvider** - Performs lookups on logged in users and maps roles via groups.
- **LDAPUserProvider** - Uses LDAP to provide information on users.
- **LDAPGroupBasedReviewerProvider** - Provides reviewers based on one or more groups
- **LDAPTeamProvider** - Provides teams based on mapping attributes to teams

## Configuration

Expand Down
8 changes: 6 additions & 2 deletions plugins/ldap/src/LDAPGroupBasedAuthzProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Role } from "@gram/core/dist/auth/models/Role.js";
import { LDAPClientSettings } from "./LDAPClientSettings.js";
import { connectLdapClient, ldapQueryOne } from "./lookup.js";
import { escapeFilterValue, getAttributeAsArray } from "./util.js";
import { DataAccessLayer } from "@gram/core/dist/data/dal.js";

export interface LDAPAuthzProviderSettings {
ldapSettings: LDAPClientSettings;
Expand All @@ -22,8 +23,11 @@ export class LDAPGroupBasedAuthzProvider
extends DefaultAuthzProvider
implements AuthzProvider
{
constructor(public settings: LDAPAuthzProviderSettings) {
super();
constructor(
dal: DataAccessLayer,
public settings: LDAPAuthzProviderSettings
) {
super(dal);
}

async getRolesForUser(sub: string): Promise<Role[]> {
Expand Down

0 comments on commit 34d9da3

Please sign in to comment.