feat: introduce user domain #287

timonmasberg · 2023-07-21T09:39:02Z

Description

Introduces a user domain with a dev in memory user store for local and preview environments, and a service that communicates with the MS Graph API to manage users on our AADB2C tenant. Furthermore, introduces a role hierarchy and a utility decorator to check the minimum required role in the request pipeline.
As a feature, if a user is logged in and gets deactivated, he will be logged out automatically.

Checklist:

The title of this PR and the commit history is conform with
the Conventional Commits specification.
I have performed a self-review of my own code.
My changes generate no new warnings, SonarCloud reports no Vulnerabilities, Bugs or Code Smells.
I have added tests (unit and E2E if user-facing) that prove my fix is effective or that my feature works,
Coverage > 80% and not less than the current coverage of the main branch.
The PR branch is up-to-date with the base branch. In case you merged main into your feature branch, make sure you have run the latest NX migrations (nx migrate --run-migrations).
I have included the reset.d.ts in the tsconfig.lib.json
I have included the reset.d.ts in the tsconfig.lib.json
I have extended the .eslintrc.json with .eslintrc.angular.json

This ensures that test users can be used by the API, the SPA and the E2E project.

# Conflicts: # package.json

libs/api/users/src/lib/infra/service/aadb2c-user.service.spec.ts

sonarqubecloud · 2023-07-21T11:02:13Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

89.6% Coverage
0.0% Duplication

# Conflicts: # .github/actions/build-and-deploy-api/action.yml # .github/workflows/next-deployment.yml # apps/api/src/.env.template # apps/spa/src/app/app.module.ts # apps/spa/src/environments/environment.ts # libs/spa/auth/src/lib/components/dev-login.component.ts # package-lock.json # package.json

JSPRH · 2024-02-14T06:42:17Z

apps/api/src/app/app.module.ts

+const IS_NEXT_DEPLOYMENT = process.env.ENVIRONMENT_NAME === 'next';
+const IS_PROD_DEPLOYMENT = process.env.ENVIRONMENT_NAME === 'prod';


How about we introduce a flag e.g., ENABLE_DEV_LOGIN and only if this is set to true we enable dev login, otherwise it is Azure Entra ID? That way we can control the use of Entra ID easier e.g. for local testing as well and use the safer method (Entra ID) by default.

JSPRH · 2024-02-14T07:07:22Z

libs/api/auth/src/lib/errors/presentable-insufficient-permission.exception.ts

@@ -0,0 +1,11 @@
+import { PresentableException } from '@kordis/api/shared';
+
+export class PresentableInsufficientPermissionException extends PresentableException {


Isn't this defacto the same as the PresentableUnauthorizedException?

apps/spa/src/app/app.module.ts

JSPRH · 2024-02-15T05:20:31Z

apps/spa-e2e/src/test-users.ts

+
+export const testUsernames = [...TEST_USERS.map((u) => u.userName)] as const;
 export type TestUsernames = (typeof testUsernames)[number];



Simplify. Accept suggestion from libs/shared/test-helpers/src/lib/test-users.test-helper.ts to enable code completion for usernames.

Suggested change

export const testUsernames = [...TEST_USERS.map((u) => u.userName)] as const;

export type TestUsernames = (typeof testUsernames)[number];

export type TestUsernames = (typeof TEST_USERS)[number]['userName'];

JSPRH · 2024-02-15T05:39:25Z

libs/api/users/src/lib/infra/controller/user.resolver.ts

+import { UserDeactivated } from './object-type/user-deactivated.object-type';
+
+@Resolver(User)
+export class UserResolver {


Can we put the assertOrgMembership to a whole resolver as a Decorator instead of ensuring that within every query/mutation?

JSPRH · 2024-02-15T06:09:24Z

libs/api/users/src/lib/infra/service/aadb2c-user.service.ts

+			} else if (key === 'role') {
+				updatePayload[`extension_${this.extensionAppId}_Role`] = value;
+			} else if (key === 'organizationId') {
+				updatePayload[`extension_${this.extensionAppId}_OrganizationId`] =


We never want to update the user's organization id, right? So if the id is not required by Azure, we should not set it here, if it is required, we have to require it in the method's signature as well.

JSPRH · 2024-02-15T06:11:58Z

libs/api/users/src/lib/infra/service/aadb2c-user.service.ts

+		await this.updateUserRequest(userId, updatePayload);
+	}
+
+	async changeEmail(userId: string, email: string): Promise<void> {


What about e-mail verification. Does Azure AD take responsibility for that?

JSPRH · 2024-02-15T06:26:09Z

libs/api/users/src/lib/infra/service/dev-user.service.ts

+ */
+@Injectable()
+export class DevUserService extends BaseUserService {
+	private readonly users: User[] = [...TEST_USERS];


If you move the type declaration for the users into the test-helpers file (probably good idea anyway to ensure correct types) and do a deep copy here, the type should be inferred automatically

JSPRH · 2024-02-15T06:33:40Z

libs/shared/test-helpers/src/lib/test-users.test-helper.ts

+import { Role } from '@kordis/shared/auth';
+
+// keep in sync with the apps/api/dev-tokens.md file!
+export const TEST_USERS = Object.freeze([


If you use as const instead of Object.freeze the values will be assumed as literals by TypeScript and enable a beautiful code completion.
Also, we should do TEST_UERS: User[] to ensure these users are all legit (especially when we change the User object).

libs/spa/auth/src/lib/components/auth.component.ts

JSPRH · 2024-02-15T14:50:00Z

libs/api/users/src/lib/infra/service/aadb2c-user.service.ts

+	}
+
+	async updateUser(
+		userId: string,


Suggested change

userId: string,

private async updateUser(

JSPRH · 2024-02-15T14:50:24Z

libs/api/users/src/lib/infra/service/aadb2c-user.service.ts

+			} else if (key === 'organizationId') {
+				updatePayload[`extension_${this.extensionAppId}_OrganizationId`] =
+					value;
+			} else {


Suggested change

} else if (key === 'organizationId') {

updatePayload[`extension_${this.extensionAppId}_OrganizationId`] =

value;

} else {

} else {

…separation of concerns

libs/api/user/src/lib/infra/service/aadb2c-user.service.spec.ts

sonarqubecloud · 2024-02-16T10:28:57Z

Quality Gate passed

Issues
2 New issues

Measures
0 Security Hotspots
86.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

timonmasberg · 2024-02-16T10:30:07Z

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

timonmasberg added 13 commits July 17, 2023 13:35

feat(spa): bootstrap graphql with angular-apollo

44b0491

feat(shared-auth): add role to auth user

6c45a73

feat(shared-test-helpers): introduce shared test helpers with test users

a186f9d

This ensures that test users can be used by the API, the SPA and the E2E project.

feat(api-users): introduces user domain

0f03940

feat(api): adapt dev tokens to org and role

fc43eaf

feat(api-shared): make kernel module global

4adf51b

refactor(api-observability): change modules to SCAM-like for easier use

5282909

refactor(api-users): change modules to SCAM-like for easier use

d375678

chore: add AADB2C secrets to next deployment

c9a4d0b

feat(spa-auth): adapt to new role property on user

3699260

chore(spa-graphql): add tests for graphql service

ce3ef37

refactor(spa): minor refactorings

5eb27fb

feat(spa): logout on user deactivated

ece00c0

timonmasberg self-assigned this Jul 21, 2023

timonmasberg changed the title ~~feat: introduce users~~ feat: introduce user domain Jul 21, 2023

Merge remote-tracking branch 'origin/main' into feat/users

2a8d2a3

# Conflicts: # package.json

github-advanced-security bot found potential problems Jul 21, 2023

View reviewed changes

timonmasberg added 3 commits July 21, 2023 11:54

fix(shared-test-helpers): add missing org ids to test users

a9f2981

fix(spa): remove oauth config from environment

048be37

chore: broader test coverage

2b12656

timonmasberg force-pushed the feat/users branch from 6a859a6 to 2b12656 Compare July 21, 2023 10:53

timonmasberg requested a review from JSPRH July 21, 2023 11:02

timonmasberg force-pushed the feat/users branch from df6b5d0 to 50566eb Compare January 20, 2024 23:35

timonmasberg force-pushed the feat/users branch from 50566eb to f615d71 Compare January 20, 2024 23:35

timonmasberg force-pushed the feat/users branch from 33a11db to 05156a7 Compare January 21, 2024 12:12

style: run prettier with new config

9b28b59

fix(spa): jest sentry error

d6089ee

timonmasberg force-pushed the main branch 2 times, most recently from 9b697ff to 99a715d Compare January 23, 2024 13:07

JSPRH requested changes Feb 15, 2024

View reviewed changes

JSPRH reviewed Feb 15, 2024

View reviewed changes

timonmasberg added 5 commits February 16, 2024 09:44

refactor(api-users): move organization check into repository restore …

2229cfb

…separation of concerns

refactor(api-user): rename users domain to singular

acc1903

chore(api-user): determine user strategy from env var

fa5e472

chore(spa-auth): add destroy handler to route query params observable

e9e9593

chore: merge

524ff24

github-advanced-security bot found potential problems Feb 16, 2024

View reviewed changes

timonmasberg added 3 commits February 16, 2024 10:59

chore: run prettier

67e733d

fix(spa-e2e): adapt to new test user changes

5529c1b

chore: pair programming findings

d77ff4c

timonmasberg enabled auto-merge (squash) February 16, 2024 10:23

JSPRH approved these changes Feb 16, 2024

View reviewed changes

timonmasberg merged commit 88966bd into main Feb 16, 2024
5 checks passed

timonmasberg deleted the feat/users branch February 16, 2024 10:29

timonmasberg added the released label Feb 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce user domain #287

feat: introduce user domain #287

timonmasberg commented Jul 21, 2023 •

edited

Loading

sonarqubecloud bot commented Jul 21, 2023

JSPRH Feb 14, 2024

JSPRH Feb 14, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

JSPRH Feb 15, 2024

sonarqubecloud bot commented Feb 16, 2024

timonmasberg commented Feb 16, 2024

		const IS_NEXT_DEPLOYMENT = process.env.ENVIRONMENT_NAME === 'next';
		const IS_PROD_DEPLOYMENT = process.env.ENVIRONMENT_NAME === 'prod';

		@@ -0,0 +1,11 @@
		import { PresentableException } from '@kordis/api/shared';

		export class PresentableInsufficientPermissionException extends PresentableException {


		export const testUsernames = [...TEST_USERS.map((u) => u.userName)] as const;
		export type TestUsernames = (typeof testUsernames)[number];

feat: introduce user domain #287

feat: introduce user domain #287

Conversation

timonmasberg commented Jul 21, 2023 • edited Loading

Description

Checklist:

sonarqubecloud bot commented Jul 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 16, 2024

Quality Gate passed

timonmasberg commented Feb 16, 2024

timonmasberg commented Jul 21, 2023 •

edited

Loading