Skip to content

Distinguish Access Rights to API routes with different roles#108

Open
dxL1nus wants to merge 5 commits into
codecheckers:mainfrom
dxL1nus:feature/api-different-access-roles
Open

Distinguish Access Rights to API routes with different roles#108
dxL1nus wants to merge 5 commits into
codecheckers:mainfrom
dxL1nus:feature/api-different-access-roles

Conversation

@dxL1nus
Copy link
Copy Markdown
Contributor

@dxL1nus dxL1nus commented Mar 24, 2026

Closes #87;

We now have a CodecheckRole class which can be extended to have our won Role Sets like e.g. ReadAccessRoles. This then gets an array of PKP\security\Role like the following:

$readAccessRole = new ReadAccessRole([
    Role::ROLE_ID_SITE_ADMIN,
    Role::ROLE_ID_MANAGER,
    Role::ROLE_ID_SUB_EDITOR,
    Role::ROLE_ID_ASSISTANT,
]);

Next an API endpoint can now have exactly one CodecheckRole Type like the following:

$this->endpoints = [
    'GET' => [
        [
            'route' => 'metadata',
            'handler' => [$this, 'getMetadata'],
            'role' => ReadAccessRole::class,
        ],
    ],
];

When the request for the endpoint is then authorized, it is checked if the endpoint role is the same as the class from each element in the $codecheckRoles array that the CodecheckApiHandler gets in its constructor. If so, then the first role element is used and its actual PKP roles are retrieved and then it is checked if the user has at least on of these roles. If so, then the route is authorized as normal.

private function getPkpRoles(string $codecheckRole): array
{
    foreach ($this->roles as $role) {
        if($role::class === $codecheckRole) {
            return $role->getRoles();
        }
    }

    throw new RoleNotFoundException("The CODECHECK Role was not found in the array of roles.");
}

and in authorize()

$pkpRoles = $this->getPkpRoles($codecheckRole);

if(!($user && $user->hasRole($pkpRoles, $contextId))) {
    $this->response->response([
        'success'   => false,
        'error'     => "User has no assigned Role or doesn't have the right roles assigned to access this resource"
    ], 400);
    return;
}

This way we can provide multiple different role sets and even create new ones, by creating new classes, which extend CodecheckRole.

@dxL1nus dxL1nus requested a review from nuest March 24, 2026 09:24
@nuest
Copy link
Copy Markdown
Member

nuest commented May 11, 2026

We need to consider here that not all users may be allowed to change the status of a check.

@dxL1nus
Copy link
Copy Markdown
Contributor Author

dxL1nus commented Jun 1, 2026

We need to consider here that not all users may be allowed to change the status of a check.

since these are 2 different PR's, I would firstly merge this, and then we can just use this functionality on the status branch

@nuest
Copy link
Copy Markdown
Member

nuest commented Jun 1, 2026

@dxL1nus Why ignore the tests?

b84d741

@dxL1nus dxL1nus force-pushed the feature/api-different-access-roles branch from b84d741 to e76afdc Compare June 1, 2026 10:52
@dxL1nus
Copy link
Copy Markdown
Contributor Author

dxL1nus commented Jun 1, 2026

@dxL1nus Why ignore the tests?

b84d741

I just did this, because this branch was so old before the rebase, that there was now test scripts in the tests directory, just the tests outcomes, which I didn't want to push of course. Now I rebase main to this branch and the gitignore is now also updated accordingly to only ignore the results in the tests directory

@dxL1nus dxL1nus marked this pull request as ready for review June 1, 2026 11:20
@nuest nuest marked this pull request as draft June 1, 2026 11:28
@nuest
Copy link
Copy Markdown
Member

nuest commented Jun 1, 2026

@dxL1nus will rebase this once more.

@dxL1nus dxL1nus force-pushed the feature/api-different-access-roles branch from c98fc2b to bf4512f Compare June 7, 2026 12:11
@dxL1nus dxL1nus marked this pull request as ready for review June 7, 2026 12:43
@dxL1nus
Copy link
Copy Markdown
Contributor Author

dxL1nus commented Jun 7, 2026

@nuest this PR is now ready for review and can be merged if you want! Everything is now rebased to main and working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Distinguish Read and Write Access with different roles to API routes

2 participants