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

Replace "User" role with "Partner" role #1628

Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ All notable changes to this project will be documented in this file. The format

- updated listing's importer to handle latest unit and priority types changes ([#1584](https://github.com/bloom-housing/bloom/pull/1584)) (Marcin Jedras)
- Sets cache manager to use Redis [#1589](https://github.com/bloom-housing/bloom/compare/dev...seanmalbert:1589/redis-cache-manager)
- removed roles for public users and assigned a "partner" role for leasing agents([#1628](https://github.com/bloom-housing/bloom/pull/1628))

- Fixed:
- Added checks for property in listing.dto transforms
Expand Down
3 changes: 2 additions & 1 deletion backend/core/src/auth/authz_policy.csv
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ p, anonymous, applicationMethod, true, read
p, admin, paperApplication, true, .*
p, anonymous, paperApplication, true, read

g, admin, user
g, admin, partner
g, partner, user
g, user, anonymous
8 changes: 8 additions & 0 deletions backend/core/src/auth/dto/user-roles.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { OmitType } from "@nestjs/swagger"
import { UserRoles } from "../entities/user-roles.entity"

export class UserRolesDto extends OmitType(UserRoles, ["isPartner", "isAdmin"] as const) {}

export class UserRolesCreateDto extends OmitType(UserRolesDto, [] as const) {}

export class UserRolesUpdateDto extends OmitType(UserRolesDto, [] as const) {}
6 changes: 1 addition & 5 deletions backend/core/src/auth/dto/user.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import { Match } from "../../shared/decorators/match.decorator"
import { passwordRegex } from "../../shared/password-regex"

export class UserDto extends OmitType(User, [
"isAdmin",
"leasingAgentInListings",
"passwordHash",
"resetToken",
"confirmationToken",
"roles",
] as const) {
@Expose()
@IsOptional()
Expand All @@ -35,10 +35,8 @@ export class UserDto extends OmitType(User, [
}

export class UserBasicDto extends OmitType(User, [
"isAdmin",
"leasingAgentInListings",
"passwordHash",
"roles",
"confirmationToken",
"resetToken",
] as const) {}
Expand All @@ -65,7 +63,6 @@ export class UserCreateDto extends OmitType(UserDto, [
"createdAt",
"updatedAt",
"leasingAgentInListings",
"roles",
] as const) {
@Expose()
@IsString({ groups: [ValidationsGroupsEnum.default] })
Expand Down Expand Up @@ -98,7 +95,6 @@ export class UserUpdateDto extends OmitType(UserDto, [
"createdAt",
"updatedAt",
"leasingAgentInListings",
"roles",
] as const) {
@Expose()
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
Expand Down
18 changes: 18 additions & 0 deletions backend/core/src/auth/entities/user-roles.entity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Column, Entity, JoinColumn, OneToOne } from "typeorm"
import { User } from "./user.entity"

@Entity({ name: "user_roles" })
export class UserRoles {
@OneToOne(() => User, (user) => user.roles, {
primary: true,
cascade: true,
})
@JoinColumn()
user: User

@Column("boolean", { default: false })
isAdmin?: boolean

@Column("boolean", { default: false })
isPartner?: boolean
}
23 changes: 6 additions & 17 deletions backend/core/src/auth/entities/user.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Entity,
Index,
ManyToMany,
OneToOne,
PrimaryGeneratedColumn,
Unique,
UpdateDateColumn,
Expand All @@ -14,7 +15,7 @@ import { IsDate, IsEmail, IsEnum, IsOptional, IsString, IsUUID, MaxLength } from
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ApiProperty } from "@nestjs/swagger"
import { Language } from "../../shared/types/language-enum"
import { UserRole } from "../enum/user-role-enum"
import { UserRoles } from "./user-roles.entity"

@Entity({ name: "user_accounts" })
@Unique(["email"])
Expand Down Expand Up @@ -86,22 +87,10 @@ export class User {
@ManyToMany(() => Listing, (listing) => listing.leasingAgents, { nullable: true })
leasingAgentInListings?: Listing[] | null

@Column("boolean", { default: false })
isAdmin: boolean

/**
* Array of roles this user can become. Logic is simple right now, but in theory this will expand to take into
* account membership in a domain (company-level or admin area level for example).
*
* In that case, this logic will likely be based on joined entities (another table/entity that keeps track of
* group membership, for example), and these relations will need to be loaded in order for the list of roles to
* work properly.
*/
@Expose()
@ApiProperty({ enum: UserRole, enumName: "UserRole", isArray: true })
get roles(): UserRole[] {
return [UserRole.user, ...(this.isAdmin ? [UserRole.admin] : [])]
}
@OneToOne(() => UserRoles, (roles) => roles.user, {
eager: true,
})
roles: UserRoles

@Column({ enum: Language, nullable: true })
@Expose()
Expand Down
3 changes: 2 additions & 1 deletion backend/core/src/auth/enum/user-role-enum.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export enum UserRole {
export enum UserRoleEnum {
user = "user",
partner = "partner",
admin = "admin",
}
11 changes: 8 additions & 3 deletions backend/core/src/auth/services/authz.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { newEnforcer } from "casbin"
import path from "path"
import { User } from "../entities/user.entity"
import { Listing } from "../../listings/entities/listing.entity"
import { UserRoleEnum } from "../enum/user-role-enum"

export enum authzActions {
create = "create",
Expand Down Expand Up @@ -39,10 +40,14 @@ export class AuthzService {

// Get User roles and add them to our enforcer
if (user) {
await Promise.all(user.roles.map((r) => e.addRoleForUser(user.id, r)))
}
if (user.roles?.isAdmin) {
await e.addRoleForUser(user.id, UserRoleEnum.admin)
}
if (user.roles?.isPartner) {
await e.addRoleForUser(user.id, UserRoleEnum.partner)
}
await e.addRoleForUser(user.id, UserRoleEnum.user)

if (user) {
// NOTE This normally should be in authz_policy.csv, but casbin does not support expressions on arrays.
// Permissions for a leasing agent on applications are there defined here programatically.
// A User becomes a leasing agent for a given listing if he has a relation (M:N) with it.
Expand Down
25 changes: 25 additions & 0 deletions backend/core/src/migration/1628543278484-partner-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {MigrationInterface, QueryRunner} from "typeorm";

export class partnerRole1628543278484 implements MigrationInterface {
name = 'partnerRole1628543278484'

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`CREATE TABLE "user_roles" ("is_admin" boolean NOT NULL DEFAULT false, "is_partner" boolean NOT NULL DEFAULT false, "user_id" uuid NOT NULL, CONSTRAINT "REL_87b8888186ca9769c960e92687" UNIQUE ("user_id"), CONSTRAINT "PK_87b8888186ca9769c960e926870" PRIMARY KEY ("user_id"))`);
// Add all partners to the table.
await queryRunner.query(`INSERT INTO "user_roles" ("user_id") SELECT DISTINCT "user_accounts_id" FROM "listings_leasing_agents_user_accounts"`)
// Add any admins to the table.
await queryRunner.query(`INSERT INTO "user_roles" ("user_id", "is_admin") SELECT "id", "is_admin" FROM "user_accounts" WHERE "user_accounts"."is_admin" = TRUE`)
// Give everyone partner permissions.
await queryRunner.query(`UPDATE "user_roles" SET "is_partner" = TRUE`)
await queryRunner.query(`ALTER TABLE "user_roles" ADD CONSTRAINT "FK_87b8888186ca9769c960e926870" FOREIGN KEY ("user_id") REFERENCES "user_accounts"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`);
await queryRunner.query(`ALTER TABLE "user_accounts" DROP COLUMN "is_admin"`);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`ALTER TABLE "user_roles" DROP CONSTRAINT "FK_87b8888186ca9769c960e926870"`);
await queryRunner.query(`ALTER TABLE "user_accounts" ADD "is_admin" boolean NOT NULL DEFAULT false`);
await queryRunner.query(`UPDATE "user_accounts" SET "is_admin" = "user_roles"."is_admin" FROM "user_roles" WHERE "user_roles"."user_id" = "user_accounts"."id"`)
await queryRunner.query(`DROP TABLE "user_roles"`);
}

}
19 changes: 14 additions & 5 deletions backend/core/src/seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { ApplicationMethodType } from "./application-methods/types/application-m
import { AuthContext } from "./auth/types/auth-context"
import { ListingDefaultReservedSeed } from "./seeds/listings/listing-default-reserved-seed"
import { ListingDefaultFCFSSeed } from "./seeds/listings/listing-default-fcfs-seed"
import { UserRoles } from "./auth/entities/user-roles.entity"

const argv = yargs.scriptName("seed").options({
test: { type: "boolean", default: false },
Expand All @@ -46,7 +47,10 @@ export function getSeedListingsCount() {
return listingSeeds.length
}

export async function createLeasingAgents(app: INestApplicationContext) {
export async function createLeasingAgents(
app: INestApplicationContext,
rolesRepo: Repository<UserRoles>
) {
const usersService = await app.resolve<UserService>(UserService)
const leasingAgents = await Promise.all(
defaultLeasingAgents.map(
Expand All @@ -55,15 +59,17 @@ export async function createLeasingAgents(app: INestApplicationContext) {
)
await Promise.all([
leasingAgents.map(async (agent: User) => {
const roles: UserRoles = { user: agent, isPartner: true }
await rolesRepo.save(roles)
await usersService.confirm({ token: agent.confirmationToken })
}),
])
return leasingAgents
}

const seedListings = async (app: INestApplicationContext) => {
const seedListings = async (app: INestApplicationContext, rolesRepo: Repository<UserRoles>) => {
const seeds = []
const leasingAgents = await createLeasingAgents(app)
const leasingAgents = await createLeasingAgents(app, rolesRepo)

const allSeeds = listingSeeds.map((listingSeed) => app.get<ListingDefaultSeed>(listingSeed))
const listingRepository = app.get<Repository<Listing>>(getRepositoryToken(Listing))
Expand Down Expand Up @@ -99,7 +105,8 @@ async function seed() {
const userService = await app.resolve<UserService>(UserService)

const userRepo = app.get<Repository<User>>(getRepositoryToken(User))
const listings = await seedListings(app)
const rolesRepo = app.get<Repository<UserRoles>>(getRepositoryToken(UserRoles))
const listings = await seedListings(app, rolesRepo)

const user1 = await userService.createUser(
plainToClass(UserCreateDto, {
Expand Down Expand Up @@ -154,8 +161,10 @@ async function seed() {
}
}

admin.isAdmin = true
await userRepo.save(admin)
const roles: UserRoles = { user: admin, isPartner: true, isAdmin: true }
await rolesRepo.save(roles)

await userService.confirm({ token: admin.confirmationToken })
await app.close()
}
Expand Down
2 changes: 2 additions & 0 deletions backend/core/src/seeder/seeder.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { AmiChart } from "../ami-charts/entities/ami-chart.entity"
import { Property } from "../property/entities/property.entity"
import { Unit } from "../units/entities/unit.entity"
import { User } from "../auth/entities/user.entity"
import { UserRoles } from "../auth/entities/user-roles.entity"
import { ListingColiseumSeed } from "../seeds/listings/listing-coliseum-seed"
import { ListingDefaultOnePreferenceSeed } from "../seeds/listings/listing-default-one-preference-seed"
import { ListingDefaultNoPreferenceSeed } from "../seeds/listings/listing-default-no-preference-seed"
Expand Down Expand Up @@ -57,6 +58,7 @@ export class SeederModule {
Property,
Unit,
User,
UserRoles,
ApplicationMethod,
PaperApplication,
]),
Expand Down
13 changes: 11 additions & 2 deletions backend/core/test/authz/authz.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ describe("Authz", () => {
await supertest(app.getHttpServer()).get(userEndpoint).expect(403)
})

it(`should not allow anonymous user to PUT user`, async () => {
await supertest(app.getHttpServer()).get(userEndpoint).expect(403)
it(`should allow a logged in user to GET to get any user profile`, async () => {
await supertest(app.getHttpServer())
.get(userEndpoint)
.set(...setAuthorization(userAccessToken))
.expect(200)
})

it(`should allow anonymous user to CREATE a user`, async () => {
Expand All @@ -114,6 +117,12 @@ describe("Authz", () => {
it("should not allow anonymous user to GET applications", async () => {
await supertest(app.getHttpServer()).get(applicationsEndpoint).expect(403)
})
it("should allow logged in user to GET applications", async () => {
await supertest(app.getHttpServer())
.get(applicationsEndpoint)
.set(...setAuthorization(userAccessToken))
.expect(200)
})
it("should not allow anonymous user to GET CSV applications", async () => {
await supertest(app.getHttpServer())
.get(applicationsEndpoint + "/csv")
Expand Down
20 changes: 14 additions & 6 deletions backend/core/types/src/backend-swagger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3411,10 +3411,18 @@ export interface LoginResponse {
accessToken: string
}

export interface User {
export interface UserRoles {
/** */
user: User

/** */
isAdmin: boolean

/** */
roles: UserRole[]
isPartner: boolean
}

export interface User {
/** */
language?: Language

Expand Down Expand Up @@ -3447,6 +3455,9 @@ export interface User {

/** */
updatedAt: Date

/** */
roles?: CombinedUserRolesTypes
}

export interface UserCreate {
Expand Down Expand Up @@ -5316,10 +5327,6 @@ export enum EnumApplicationsApiExtraModelOrder {
"ASC" = "ASC",
"DESC" = "DESC",
}
export enum UserRole {
"user" = "user",
"admin" = "admin",
}
export enum EnumListingFilterParamsComparison {
"=" = "=",
"<>" = "<>",
Expand Down Expand Up @@ -5378,5 +5385,6 @@ export type CombinedApplicationMailingAddressTypes = AddressUpdate
export type CombinedImageTypes = AssetCreate
export type CombinedLeasingAgentAddressTypes = AddressUpdate
export type CombinedResultTypes = AssetCreate
export type CombinedUserRolesTypes = UserRoles
export type CombinedWhatToExpectTypes = WhatToExpect
export type CombinedJurisdictionTypes = Id
8 changes: 4 additions & 4 deletions sites/partners/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
LocalizedLink,
} from "@bloom-housing/ui-components"
import moment from "moment"
import { UserRole, Listing } from "@bloom-housing/backend-core/types"
import { Listing } from "@bloom-housing/backend-core/types"
import { AgGridReact } from "ag-grid-react"
import { GridOptions } from "ag-grid-community"

Expand All @@ -20,7 +20,7 @@ import { MetaTags } from "../src/MetaTags"
export default function ListingsList() {
const { profile } = useContext(AuthContext)
const leasingAgentInListings = profile.leasingAgentInListings?.map((item) => item.id)
const isAdmin = profile.roles.includes(UserRole.admin)
const isAdmin = profile.roles?.isAdmin || false
class formatLinkCell {
link: HTMLAnchorElement

Expand Down Expand Up @@ -136,7 +136,7 @@ export default function ListingsList() {
const { listingDtos, listingsLoading, listingsError } = useListingsData()
// filter listings to show items depends on user role
const filteredListings = useMemo(() => {
if (profile.roles.includes(UserRole.admin)) return listingDtos
if (isAdmin) return listingDtos

return listingDtos?.reduce((acc, curr) => {
if (leasingAgentInListings.includes(curr.id)) {
Expand All @@ -145,7 +145,7 @@ export default function ListingsList() {

return acc
}, []) as Listing[]
}, [leasingAgentInListings, listingDtos, profile.roles])
}, [leasingAgentInListings, listingDtos, isAdmin])

if (listingsLoading) return "Loading..."
if (listingsError) return "An error has occurred."
Expand Down
4 changes: 1 addition & 3 deletions sites/partners/src/ListingGuard.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useContext } from "react"
import { useRouter } from "next/router"
import { AuthContext } from "@bloom-housing/ui-components"
import { UserRole } from "@bloom-housing/backend-core/types"

type AuthGuardProps = {
children: React.ReactElement
Expand All @@ -14,8 +13,7 @@ const ListingGuard = ({ children }: AuthGuardProps) => {
const { profile } = useContext(AuthContext)

const leasingAgentInListingsIds = profile.leasingAgentInListings.map((item) => item.id)
const hasPrivileges =
profile.roles.includes(UserRole.admin) || leasingAgentInListingsIds.includes(listingId)
const hasPrivileges = profile.roles?.isAdmin || leasingAgentInListingsIds.includes(listingId)

if (hasPrivileges) {
return children
Expand Down
Loading