OLMIS-3135: Fixed problem with obtaining access token

Activity

FEOLMIS-2304 29

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 1h 47m 16 I marked the root comment as resolved and closed this rev...
    Reviewer - 92% reviewed 24m 6 does not allow using
    Reviewer - Complete 23m 3 Yea now I see, I'm starting to use it in my changes too
    Reviewer - 100% reviewed 34m 1 So I'm wanting to understand what is happening here. Tell...
    Reviewer - 100% reviewed 15m 2 I'm a little lost in all these reviews. It would be good ...
    Total   3h 22m 29  
    #permalink

    Objectives

    There are no specific objectives for this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Chongsun Ahn

    So I'm wanting to understand what is happening here. Tell me if this is corre...

    So I'm wanting to understand what is happening here. Tell me if this is correct.

    I guess when a service account is being created:

    • reference data generates an access token by consulting auth
    • reference data POSTs to /apiKeys (in auth) with the access token
    • auth then creates an oauth client, with auto-generated id and secret, and returns back the token
    • reference data then saves the token with creation details to the service account and returns back both


    If my understanding is correct, why does reference data need to consult auth to generate an access token, especially since it seems like reference data is passing the token to auth anyways? Especially because it seems auth ignores the access token that is being passed into the request body on the POST /apiKeys?

    Łukasz Lewczyński

    Not exactly. When a user creates a service account: *The controller checks ...

    Not exactly. When a user creates a service account:

    • The controller checks if the user has permission
    • It calls the auth endpoint (without any data)
      • The auth creates client details
      • It generates access_token for the given client id
      • it returns the token to the reference data service
    • A new instance of service account with the given access token as API key in created
    • It saves the instance to database
    • The instance is returned to the user.


    The service account is divided into two microservices basically because from my understanding in the future we will add some rights/roles to service accounts and those data are in the reference data. Also before we create API key we need related client details and this can be created only in the auth service.

    We need new client details for each API key because they are always valid and they don't expire. If we had only one client detains, we would have only one API key because the Spring OAuth would return it. This is how the OAuth works. Firstly it try to find a access_token in database for the given parameters and if it exists and it is not expired, it will be returned.

    Paweł Albecki

    I'm a little lost in all these reviews. It would be good to document how it a...

    I'm a little lost in all these reviews. It would be good to document how it all work and update https://openlmis.atlassian.net/wiki/spaces/OP/pages/109772828/Auth+Service+Documentation

    Łukasz Lewczyński

    Sebastian Brudziński I updated the docs.

    Sebastian Brudziński I updated the docs.

    Łukasz Lewczyński

    Also the DESIGN.md has been updated

    Also the DESIGN.md has been updated

    Josh Zamor

    Two topics from todays showcase I'd like to follow up on: 1. Reference data...

    Two topics from todays showcase I'd like to follow up on:

    1. Reference data having a dependency on Auth (this call from referencedata controller to auth)
    2. What level of permissions this new token gives, is it root for /all/ endpoints?


    Where can I best followup on those questions?

    Łukasz Lewczyński

    1. Yes currently there is a dependency between reference data and auth servic...

    1. Yes currently there is a dependency between reference data and auth services. I am not sure how to cut this dependency because roles, rights are in the reference data but client details and access tokens are in the auth service. I am start thinking that maybe we should move roles, rights, role assignment to the auth service. I am not sure why those entities are in the reference data service. Those entities are related with authentication and the auth service is responsible for that.

    One more think current approach is safe because user call only one endpoint and get response if we divide it to two requests, there could some security issues. I think similar problem is with creating an user because if I understood correctly the code I am able to create several users in the auth service that will be related with single user in the reference data service because we don't check if there is one-to-one relation between those two user entities.

    2. Currently the API key is treated in the same way as the service-based token. My understanding is that in the future there will be a list of rights/roles assigned to the API Key (aka Service Account) according to this assumption from the ticket Service accounts may be granted administrative rights (Josh Zamor - removed this from the acceptance criteria unless it can be defined before starting Sprint 41. Otherwise this can be a separate ticket.)

    Łukasz Lewczyński

    I think I found workaround if we wanted to leave users, roles, rights and oth...

    I think I found workaround if we wanted to leave users, roles, rights and other entities in the reference data service.

    The solution will be similar to how we handle users entities. There will an entity in the auth service that will contain an API key, creator id and creation date. The ID of this entity will be used to create an entity in the reference data service (it will be used as a wrapper that will contain the ID and list of rights/roles). Without this the API key will be useless because we would add entity ID to the check_token endpoint response (like referenceDataUserId field) and use it to check if the given API key has assigned the required right. In other words:

    1. To generate API key by Endpoints:

    • call endpoint in the auth service - it will return an entity with id, apiKey and creation details
    • call endpoint in the reference-data service with ID from the previous response


    2. To generate API key by UI:

    • User clicks on add button on ui. Other thinks would be handled by JS


    3. Code changes:

    • code changes related with how entities are saved by services (see #1 and #2)
    • modify the AccessTokenEnhancer class in the auth service to add id of entity related with API key.
      • we could check if token is related with API key by checking if authentication.getOAuth2Request().getClientId() starts with proper prefix.
    • in the PermissionService class in each services (currently we have about 6 of them) we use ID to check if API key has assigned the required right (this will be done in the reference data service, similar to hasRight endpoint for users)


    In the end the auth service will contain API keys, the reference data service will know only ID of API keys so there will not be a call from the reference data service to the auth service.

    cc: Chongsun Ahn Paweł Albecki Nikodem Graczewski Sebastian Brudziński

    Łukasz Lewczyński

    Endpoints: (Auth) POST /api/apiKeys - generate a new API key (Auth) DELETE /...

    Endpoints:

    (Auth) POST /api/apiKeys - generate a new API key
    (Auth) DELETE /api/apiKeys/{id} - delete the API key related with ID
    (Auth) GET /api/apiKeys/ - get a page of API Keys

    (Reference Data) POST /api/serviceAccounts BODY { "apiKeyId": "<<uuid>>" } - generate related Service account
    (Reference Data) GET /api/serviceAccounts/{apiKeyId}/ - get service account based on api key id
    (Reference Data) DELETE /api/serviceAccounts/{apiKeyId} - delete service account related with id
    (Reference Data) POST /api/serviceAccounts/{apiKeyId} - update rights/roles for service account
    (Reference Data) GET /api/serviceAccounts/{apiKeyId}/hasRight?facilityId=&programId=&warefouse= - check if API key has right to facility, program and/or warehouse

    Łukasz Lewczyński

    I marked the root comment as resolved and closed this review. The changes rel...

    I marked the root comment as resolved and closed this review. The changes related with those comments are in FEOLMIS-2314 and FEOLMIS-2313. Also adding role assigments to the API keys was moved to OLMIS-3861

    /src/.../web/ApiKeyControllerIntegrationTest.java Changed
    /src/main/java/.../auth/domain/Client.java Changed
    /src/main/.../repository/ClientRepository.java Changed 3
    /src/.../consul/ConsulCommunicationService.java Changed 2
    /src/main/.../service/AccessTokenService.java Changed 2
    /src/.../service/BaseCommunicationService.java Changed
    /src/main/.../auth/web/ApiKeyController.java Changed
    /src/main/resources/application.properties Changed
    /src/.../service/AccessTokenServiceTest.java Changed
    /src/.../service/BaseCommunicationServiceTest.java Changed
    /src/test/.../auth/ClientDataBuilder.java Added
    Open in IDE #permalink
    /DESIGN.md Changed 12

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against