guardian icon indicating copy to clipboard operation
guardian copied to clipboard

Decouple access management from appeal

Open rahmatrhd opened this issue 2 years ago • 1 comments

Summary

Currently, appeal acts as the information holder for approval flow and user access. We can decouple those responsibilities by creating a new entity called access that manages user access and its lifecycle.

Changes:

1. Process

  1. Once appeal is approved, guardian will create an access entry representing the current user access to the resource
  2. Access revocation will be done to access instead of appeal
  3. If a new appeal to the same resource created by the same user with the same role is approved, guardian will revoke the existing active access (if exists) and create a new one.

2. Entity

Appeal:

  type Appeal struct {
  	ID            string                 
  	ResourceID    string                 
  	PolicyID      string                 
  	PolicyVersion uint                   
  	Status        string                 
  	AccountID     string                 
  	AccountType   string                 
  	CreatedBy     string                 
  	Creator       interface{}            
  	Role          string                 
  	Permissions   []string               
  	Options       *AppealOptions         
  	Details       map[string]interface{} 
  	Labels        map[string]string      

- 	RevokedBy    string    
- 	RevokedAt    time.Time 
- 	RevokeReason string    

  	Policy    *Policy     
  	Resource  *Resource   
  	Approvals []*Approval 

  	CreatedAt time.Time 
  	UpdatedAt time.Time 
  }

Access:

+  type Access struct {
+ 	ID             string
+ 	Status         string // active | revoked
+ 	AccountID      string
+ 	AccountType    string
+ 	ResourceID     string
+ 	Permissions    []string
+ 	ExpirationDate *time.Time
+ 	AppealID       string
+ 	RevokedBy      string
+ 	RevokedAt      *time.Time
+ 	RevokeReason   string
+ 	CreatedAt      time.Time
+ 	UpdatedAt      time.Time
+  }

3. Lifecycle

Appeal:

  1. pending
    2. canceled
-   2. active
+   2. approved
-     3. terminated
    2. rejected

Access:

+ 1. active
+   2. revoked

rahmatrhd avatar Aug 04 '22 10:08 rahmatrhd

@rahmatrhd does the mean of *time.Time is a nullable time? So is it possible to have an access with null ExpirationDate ?

mabdh avatar Aug 14 '22 00:08 mabdh

@mabdh yes, that's the case for permanent access

rahmatrhd avatar Aug 14 '22 17:08 rahmatrhd

@bsushmith and I were having some discussions on invite based providers, suppose a user requesting for a resource didn’t accept the invite within the given the time(24 hours), should we make him create the appeal on guardian again? In that case the managers/resource owners will have to approve the appeal again for the same user.

We were thinking if there’s a possibility to ensure that if the approvers have already approved the user's appeal, and the invite has expired, can we keep a boolean flag (verified) in Access Struct to check on that. Using this we can make another API call to resend the invite without going into the entire approval process again? This would simplify the invite based flow, instead of making the users and approvers do the same thing twice. @rahmatrhd @AkarshSatija @ravisuhag @mabdh

Chief-Rishab avatar Aug 15 '22 05:08 Chief-Rishab

@bsushmith @Chief-Rishab I was thinking that guardian can have a job that periodically checks the invitation status to the provider and set the access status to active or canceled accordingly when the access created with initial status pending

rahmatrhd avatar Aug 15 '22 06:08 rahmatrhd

in order to rollout this access entity while still maintaining the backward compatibility for the client (frontend), we need to have two separate releases/versions:

Release 1

  • introduce access entity
  • introduce GET /accesses, GET /me/accesses, GET /accesses/:id, PUT /accesses/:id/revoke, and PUT /accesses/revoke (bulk revoke accesses) APIs
  • deprecate PUT /appeals/:id/revoke, and POST /appeals/revoke (bulk revoke appeals) APIs
  • maintain the status of provider access in both appeal and access (see the table below for the mapping)
appeal status access status business logic changes db migration
pending N/A - -
canceled N/A - -
rejected N/A - -
active active create access entry when the appeal is activated active appeals will be migrated to active access
terminated inactive revoking active appeal or access will change the status of both entities, appeal -> terminated, access -> inactive terminated appeal records will be migrated to inactive access records along with the revoked_at, revoked_by, and revoke_reason information

Release 2

  • delete PUT /appeals/:id/revoke, and POST /appeals/revoke (bulk revoke appeals) APIs
  • only maintain the status of provider access in access (see the table below for the mapping)
appeal status access status business logic changes db migration
pending N/A - -
canceled N/A - -
rejected N/A - -
approved active change appeal status to approved instead of active when all approvals got approved update appeals with status = active to approved
approved inactive remove appeal revoke logic from accessService.Revoke and accessService.BulkRevoke update appeals with status = terminated to approved and remove revoked_at, revoked_by, and revoke_reason columns from appeals

In between those two releases we can update the client (frontend) to use access instead of appeal for listing list of user accesses

@mabdh @Chief-Rishab @ravisuhag @bsushmith @singhvikash11 please have a look and let me know if there's something I missed

rahmatrhd avatar Aug 23 '22 16:08 rahmatrhd

In between those two releases we can update the client (frontend) to use access instead of appeal for listing list of user accesses

Why can't this be part of Release 1 ?

mabdh avatar Aug 24 '22 02:08 mabdh

@mabdh the releases I mentioned are specific for guardian (backend), updating the client going to be a separate task. But still, release1, update client, and release2 had to be done sequentially as each depends on the prior task

rahmatrhd avatar Aug 24 '22 03:08 rahmatrhd

@rahmatrhd based on what you mention, updated client should be part of release 2 then?

mabdh avatar Aug 24 '22 03:08 mabdh

@mabdh the client (frontend) now is managed in different project/repo anyway, so the requirement for guardian is only to have two separate releases/versions with requirements I mentioned above. This strategy is basically to give the client chance to update to the new version without breaking/having downtime during the backend is using release1, and release2 is basically to remove the deprecated/unused features.

rahmatrhd avatar Aug 24 '22 04:08 rahmatrhd

Oh I see got it, by client, I thought it is guardian cli.

mabdh avatar Aug 24 '22 04:08 mabdh

(As discussed with @rahmatrhd ) Guardian CLI commands will also have to be updated in release 2

Chief-Rishab avatar Aug 25 '22 04:08 Chief-Rishab