[ 
https://issues.apache.org/jira/browse/AIRAVATA-2908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Marcus Christie updated AIRAVATA-2908:
--------------------------------------
    Description: 
Profile Service methods are annotated with @SecurityCheck but the Profile 
Service isn't configured with an interceptor like AiravataServerHandler: 
https://github.com/apache/airavata/blob/970dc68e659daf8d633e7aaeb6dceaac063d6725/airavata-api/airavata-api-server/src/main/java/org/apache/airavata/api/server/AiravataAPIServer.java#L186-L186,
 so it isn't really doing anything.

But just wiring up the interceptor won't work. We need to rethink what 
@SecurityCheck is doing. The KeyCloakSecurityManager is written such that it 
whitelists several API server methods, but that approach won't scale if we 
start adding other services.

I would like to see @SecurityCheck/SecurityInterceptor do the following:
* validate the token and store that in the cache up to the token expiration time
* if an API method is annotated with {{@SecurityCheck}} then it simply checks 
the token
* if an API method is annotated with {noformat}@SecurityCheck(groups={ADMIN, 
READ_ONLY_ADMIN}){noformat} then it checks the GatewayGroups to see if the user 
is in one of those groups

This will allow us to simplify KeyCloakSecurityManager: it won't need to have a 
whitelist of API methods and the group based auth on an API method is 
configured right on the method.

For example, in API server
{code:java}
    @Override
    @SecurityCheck(groups={ADMIN})
    public boolean deleteComputeResource(AuthzToken authzToken, String 
computeResourceId) throws InvalidRequestException,
            AiravataClientException, AiravataSystemException, 
AuthorizationException, TException {
    ...
    }
{code}

Some other thoughts:
* NOTE: the Profile Service may end up being broken out into a separate 
security project ("Custos"), so this may not be necessary. However, there are 
definite benefits to the API server too and future public APIs
* We could keep going with this and have @SecurityCheck also check the sharing 
registry. We would need to annotate the API method parameter that is the entity 
id and also provide the required permission, for example:
{code:java}
    @Override
    @SecurityCheck(permission=READ)
    public ExperimentModel getExperiment(AuthzToken authzToken, 
@SharingEntityId String airavataExperimentId) throws InvalidRequestException,
            ExperimentNotFoundException, AiravataClientException, 
AiravataSystemException, AuthorizationException, TException {
...
}
{code}

  was:
Profile Service methods are annotated with @SecurityCheck but the Profile 
Service isn't configured with an interceptor like AiravataServerHandler: 
https://github.com/apache/airavata/blob/970dc68e659daf8d633e7aaeb6dceaac063d6725/airavata-api/airavata-api-server/src/main/java/org/apache/airavata/api/server/AiravataAPIServer.java#L186-L186,
 so it isn't really doing anything.

But just wiring up the interceptor won't work. We need to rethink what 
@SecurityCheck is doing. The KeyCloakSecurityManager is written such that it 
whitelists several API server methods, but that approach won't scale if we 
start adding other services.

I would like to see @SecurityCheck/SecurityInterceptor do the following:
* validate the token and store that in the cache up to the token expiration time
* if an API method is annotated with {{@SecurityCheck}} then it simply checks 
the token
* if an API method is annotated with {{@SecurityCheck(groups={ADMIN, 
READ_ONLY_ADMIN})}} then it checks the GatewayGroups to see if the user is in 
one of those groups

This will allow us to simplify KeyCloakSecurityManager: it won't need to have a 
whitelist of API methods and the group based auth on an API method is 
configured right on the method.

For example, in API server
{code:java}
    @Override
    @SecurityCheck(groups={ADMIN})
    public boolean deleteComputeResource(AuthzToken authzToken, String 
computeResourceId) throws InvalidRequestException,
            AiravataClientException, AiravataSystemException, 
AuthorizationException, TException {
    ...
    }
{code}

Some other thoughts:
* NOTE: the Profile Service may end up being broken out into a separate 
security project ("Custos"), so this may not be necessary. However, there are 
definite benefits to the API server too and future public APIs
* We could keep going with this and have @SecurityCheck also check the sharing 
registry. We would need to annotate the API method parameter that is the entity 
id and also provide the required permission, for example:
{code:java}
    @Override
    @SecurityCheck(permission=READ)
    public ExperimentModel getExperiment(AuthzToken authzToken, 
@SharingEntityId String airavataExperimentId) throws InvalidRequestException,
            ExperimentNotFoundException, AiravataClientException, 
AiravataSystemException, AuthorizationException, TException {
...
}
{code}


> Improve SecurityCheck, generalize it, and apply to Profile Service API methods
> ------------------------------------------------------------------------------
>
>                 Key: AIRAVATA-2908
>                 URL: https://issues.apache.org/jira/browse/AIRAVATA-2908
>             Project: Airavata
>          Issue Type: Improvement
>            Reporter: Marcus Christie
>            Priority: Major
>
> Profile Service methods are annotated with @SecurityCheck but the Profile 
> Service isn't configured with an interceptor like AiravataServerHandler: 
> https://github.com/apache/airavata/blob/970dc68e659daf8d633e7aaeb6dceaac063d6725/airavata-api/airavata-api-server/src/main/java/org/apache/airavata/api/server/AiravataAPIServer.java#L186-L186,
>  so it isn't really doing anything.
> But just wiring up the interceptor won't work. We need to rethink what 
> @SecurityCheck is doing. The KeyCloakSecurityManager is written such that it 
> whitelists several API server methods, but that approach won't scale if we 
> start adding other services.
> I would like to see @SecurityCheck/SecurityInterceptor do the following:
> * validate the token and store that in the cache up to the token expiration 
> time
> * if an API method is annotated with {{@SecurityCheck}} then it simply checks 
> the token
> * if an API method is annotated with {noformat}@SecurityCheck(groups={ADMIN, 
> READ_ONLY_ADMIN}){noformat} then it checks the GatewayGroups to see if the 
> user is in one of those groups
> This will allow us to simplify KeyCloakSecurityManager: it won't need to have 
> a whitelist of API methods and the group based auth on an API method is 
> configured right on the method.
> For example, in API server
> {code:java}
>     @Override
>     @SecurityCheck(groups={ADMIN})
>     public boolean deleteComputeResource(AuthzToken authzToken, String 
> computeResourceId) throws InvalidRequestException,
>             AiravataClientException, AiravataSystemException, 
> AuthorizationException, TException {
>     ...
>     }
> {code}
> Some other thoughts:
> * NOTE: the Profile Service may end up being broken out into a separate 
> security project ("Custos"), so this may not be necessary. However, there are 
> definite benefits to the API server too and future public APIs
> * We could keep going with this and have @SecurityCheck also check the 
> sharing registry. We would need to annotate the API method parameter that is 
> the entity id and also provide the required permission, for example:
> {code:java}
>     @Override
>     @SecurityCheck(permission=READ)
>     public ExperimentModel getExperiment(AuthzToken authzToken, 
> @SharingEntityId String airavataExperimentId) throws InvalidRequestException,
>             ExperimentNotFoundException, AiravataClientException, 
> AiravataSystemException, AuthorizationException, TException {
> ...
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to