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

Update AuthorizationFactory interface #1704

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Update AuthorizationFactory interface #1704

merged 2 commits into from
Sep 5, 2024

Conversation

tylerwowen
Copy link
Contributor

@tylerwowen tylerwowen commented Aug 27, 2024

Update AuthorizationFactory interface to make it more secure.

The issues

Wrong method invoked during on-the-fly authorization

TeletraanAuthorizer<TeletraanPrincipal> authorizer = authorizationFactory.create(context);

When CompositeAuthorizationFactory is configured, this always returns the BasePastisAuthorizer. It is a problem because ScriptTokenPrincipal can only be authorized by ScriptTokenRoleAuthorizer, otherwise illegal access is possible. It was prevented before the introduction of on-the-fly authorization via bundling the authenticator and the authorizer in the auth filter creation.

AuthorizationFactory interface

The interface of AuthorizationFactory makes it possible get wrong authorizer.

<P extends TeletraanPrincipal> TeletraanAuthorizer<P> create(TeletraanServiceContext context)
            throws Exception;

Fixes

Dissociate TeletraanAuthorizer and Authorizer interfaces.

TeletraanAuthorizer stops extending Authorizer. All authorizers implementing TeletraanAuthorizer now need to implement TeletraanAuthorizer and Authorizer respectively.

New method in AuthorizationFactory

The existing methods are updated to return Authorizer.

<P extends TeletraanPrincipal> Authorizer<P> create(TeletraanServiceContext context);

Introduced a new method to return TeletraanAuthorizer.

Call new method in EnvCapacities

authorizationFactory.createSecondaryAuthorizer(
                        context, teletraanPrincipal.getClass());

And it's the only option now, no way to be mistaken.

@github-actions github-actions bot added the deploy-service Includes changes to deploy-service label Aug 27, 2024
@tylerwowen tylerwowen changed the title Touyang/interface Update AuthorizationFactory interface Aug 27, 2024
@tylerwowen tylerwowen marked this pull request as ready for review August 27, 2024 23:41
@tylerwowen tylerwowen requested a review from a team as a code owner August 27, 2024 23:41
@tylerwowen tylerwowen requested a review from osoriano August 27, 2024 23:41
@tylerwowen tylerwowen merged commit 862458a into master Sep 5, 2024
6 checks passed
@tylerwowen tylerwowen deleted the touyang/interface branch September 5, 2024 22:38
tylerwowen added a commit that referenced this pull request Sep 13, 2024
This is a follow up PR for #1704.

We can allow ScriptTokenPrincipal for ad-hoc authorization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-service Includes changes to deploy-service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants