BC-8137 - move auth rules into modules #5271
Conversation
This reverts commit 8408a74.
This reverts commit dd1909a.
* update school rule * migration: Migration20240925165112 --------- Co-authored-by: Alexander Weber <[email protected]>
apps/server/src/modules/authorization-reference/domain/authorization-reference.service.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/authorization-reference/domain/reference.loader.spec.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/authorization-reference/domain/reference.loader.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/authorization-reference/domain/reference.loader.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/authorization-reference/domain/reference.loader.ts
Show resolved
Hide resolved
|
|
||
| @Injectable() | ||
| export class TeamAuthorisableService implements AuthorizationLoaderServiceGeneric<TeamEntity> { | ||
| constructor(private readonly teamsRepo: TeamsRepo) {} | ||
| constructor(private readonly teamsRepo: TeamsRepo, injectionService: AuthorizationInjectionService) { |
There was a problem hiding this comment.
Hier ist auch domain vs api layer ein Problem.
| @@ -3,9 +3,10 @@ import { TeamsRepo } from '@shared/repo'; | |||
| import { LoggerModule } from '@src/core/logger'; | |||
| import { CqrsModule } from '@nestjs/cqrs'; | |||
| import { TeamAuthorisableService, TeamService } from './service'; | |||
| import { AuthorizationModule } from '../authorization'; | |||
| user, | ||
| role, | ||
| const user: User = userFactory.build({ roles: [role], school }); | ||
| return { |
| ], | ||
| providers: [ | ||
| ContextExternalToolService, | ||
| ContextExternalToolValidationService, | ||
| ContextExternalToolAuthorizableService, | ||
| ToolReferenceService, | ||
| ToolConfigurationStatusService, | ||
| ContextExternalToolRule, |
There was a problem hiding this comment.
Ich schätze in den Modulen ist auch keine saubere Grenze zwichen api und domain Module gewahrt. Bzw. das api war bisher nicht vorhanden da der api Part (die Rule) sich im Authorization Module versteckt hatte.
Ich finde es allerdings unglücklich das jetzt im Domain Module zu platzieren. Das räumt später nie wieder einer auf und wird im schlimmsten Fall noch adaptiert.
Edit: Ich könnte schwören das wir in irgendein Team Review mal angemerkt hatten, das contextExternalToolAuthorizableService aus dem context-external-tool.module.ts raus muss.
Das zieht sich dann so durch im Tool Module.
| import { SchoolExternalTool } from '../domain'; | ||
|
|
||
| @Injectable() | ||
| export class SchoolExternalToolAuthorizableService implements AuthorizationLoaderServiceGeneric<SchoolExternalTool> { |
There was a problem hiding this comment.
AuthorizationLoaderServiceGeneric sollte eigentlich auch bereits entfernt werden und durch AuthorizationLoaderService ersetzt werden. Ich sehe erstmal kein Grund das sich bei Tools nicht direkt verwenden lässt. Da die Domain Objekte alle von AuthorizableObject extenden.
Hier wäre schön wenn du noch mal schauen könntest, ob sich das direkt überall in Tools austauschen lässt.
Edit: Eigentlich sogar in allen Bereichen das ist ja jetzt sogar in group, instance und course-do.service gelandet...
Evtl. aber eine Arbeit für ein extra Ticket, sicher schnell gemacht. Aber anderer Fokus.
Todo: Ticket erstellen, wer erstellt es?
hoeppner-dataport
changed the title
|
Description
Links to Tickets or other pull requests
Changes
Datasecurity
Deployment
New Repos, NPM pakages or vendor scripts
Approval for review
generate-client:serverwas executed in vue frontend and changes were tested and put in a PR with the same branch name.