Import pre-existing access from providers #206

Chief-Rishab · 2022-07-07T09:53:56Z

Requirements

Guardian to collect all pre-existing access from each resource in the provider
User (admin) to be able to revoke that imported access if needed

Approach:

1. Fetching access

~~Option 1: add a flag in the provider config to import access~~
Pros : Under the assumption that after the provider is onboarded to guardian there will be no access created outside guardian, this approach would be just sufficient (simpler process)
Cons : Will only run once when the provider is just created
Option 2: provide an API endpoint to trigger import access
Pros : Can be triggered at any time
Cons : Need to be triggered manually
~~Option 3: regularly collect existing access with jobs~~
Pros : Continuously control those access created outside guardian
Cons : Might increase the scope to the Track access drift feature

2. Creating the appeals

The appeal going to be like this:

{
  "id": "", // auto-generated
  "resource_id": "", // added by guardian
  "policy_id": null,
  "policy_version": 0,
  "status": "active",
  "account_type": "", // imported from provider
  "account_id": "", // imported from provider
  "role": "custom", // TODO: need to find a way to map the existing/imported permissions with the roles user defined in the provider
  "options": {},
  "resource": {}, // added by guardian,
  "approvals": [], // depends on the policy
  "details": {},
  "created_by": "", // TODO: for "user" account type, we can use that as the value here, but can't for serviceAccount or other account types
  "creator": null, // depends on the policy. Might be empty because iam config defined in the policy

  // new field(s):
  "imported": true, // imported flag to differentiate with normal (user-created) appeal
  "permissions": [] // https://github.com/odpf/guardian/issues/205 
}

Things to note

Policy

Policy is going to be null since there's no approval flow for the imported access

Account Types

We are going to import pre-existing access for account types that are defined in the allowed_account_types field in the provider config only.

Role

Assuming this bug has been resolved,

Case 1: the imported access are predefined roles in the Provider Config
1. Suppose in the provider config we have defined a role named bq-admin which has two permissions: roles/bigquery.dataViewer and roles/bigquery.dataEditor.
2. In case a user has following access granted outside Guardian: roles/bigquery.dataViewer and roles/bigquery.dataEditor
3. Those access will be imported and mapped as an appeal with role:bq-admin only. Note that it will also have "permissions": ["roles/bigquery.dataViewer", "roles/bigquery.dataEditor"] in the appeal object
Case 2: the imported access don't have permissions defined in the provider config
1. A user has roles/bigquery.metadataViewer access in the bigquery, but that permission was not defined in the provider config.
2. In that case, each access will be mapped into a single appeal with "role" = "roles/bigquery.metadataViewer" and "permissions" = ["roles/bigquery.metadataViewer"]

The text was updated successfully, but these errors were encountered:

rahmatrhd · 2022-07-07T09:54:58Z

@ravisuhag @AkarshSatija @mabdh @bsushmith @singhvikash11 need your help to go through this rfc 🙏

ravisuhag · 2022-07-07T09:57:29Z

Draft PR which addresses this issues. #167

ravisuhag · 2022-07-07T10:00:52Z

@rahmatrhd
I think option 2 is better to start with. we can have an API to import access which can be triggered manually and a corresponding cli command e.g.guardian provider import <provide-name> -r <resources>

Flag to identify whether it is imported or not can be generalized. maybe something like. grant: import/policy. There can be a better name for it.

singhvikash11 · 2022-07-08T08:49:02Z

@Chief-Rishab I think adding no policies or no approvers associated with these appeals would not be ideal. There should be an admin page and the admin/approver could revoke this access from users later.

@rahmatrhd let's set up some time to discuss RFC, we can catch up and go through it.

rahmatrhd · 2022-07-11T03:00:42Z

There should be an admin page and the admin/approver could revoke this access from users later.

@singhvikash11 currently anyone with the revoke API access can revoke any appeals, it's not depending on the policy. Apart from that, a policy is being used to determine the approvers, while those imported access are already active access, so we no longer have to set the approvers.

rahmatrhd · 2022-07-29T05:24:12Z

found an edge case: since provider resources are not synced to guardian in real time, when importing existing access from provider, there might be a chance it contains access from newly created resources that haven't been synced to guardian yet. Those access can't be added to guardian unless we also add the new resources to the guardian which will add a side-effect to import access api and makes it not clean (first option). Or the easiest one is, to ignore the access from resources that haven't been synced to guardian.
cc @ravisuhag @bsushmith @singhvikash11 @Chief-Rishab

Chief-Rishab · 2022-07-29T05:42:38Z

Can we make a cron job/API to regularly fetch resources first rather than ignoring resources not synced with Guardian.? That would increase the scope for drift management as well but will be good for the end user

ravisuhag · 2022-07-29T05:58:16Z

@rahmatrhd As part of import access, we should ignore the access of resources that are not registered in Guardian. The drift of resources can be handled separately.

rahmatrhd · 2022-08-04T04:57:25Z

Have thought of merging this import access with the existing FetchResources process since for import access we are doing it for every resource in the provider. We can rename FetchResources to Sync and what happen in that is we are syncing the resources and the access. In this case, since fetch resources already happen through a job, then option 1 and 2 will be implemented altogether. The API that initially only for import access could become /providers/:id/sync (still can have option to sync resources only or access only)

Reason I come with this proposal is, when fetching access from provider we are also fetching the resources again to get the access entries for each. And I think it could be a signal that these process can be merged together

rahmatrhd · 2022-08-04T07:38:53Z

as discussed, we will keep both processes decoupled until we see better use cases or requirements for that.

Import Access will still be done through an API but user can add filter for resources: GET /providers/:id/import-access?resource_types=t1&resource_types=t2&resource_urns=r1. Or if user wants to import access for all resources query ?all=true also can be added.

The function signature for ImportAccess in the provider client would be like this: ImportAccess(*domain.ProviderConfig, []*domain.Resource) error (not final) to give flexibility for user to filter the resources and future use cases

ravisuhag · 2022-08-21T17:08:50Z

In addition to this, at provider level provider will provide a method that takes resources as input and import access for the provided resources.

ravisuhag added this to Roadmap Jul 7, 2022

ravisuhag added the roadmap label Jul 7, 2022

ravisuhag moved this to 2022 Q3 in Roadmap Jul 7, 2022

ravisuhag added the guardian label Jul 7, 2022

singhvikash11 closed this as completed Jul 8, 2022

singhvikash11 reopened this Jul 8, 2022

ravisuhag linked a pull request Jul 8, 2022 that will close this issue

feat: initial implementation of metabase group migration into guardian #167

Draft

ravisuhag removed the guardian label Jul 13, 2022

rahmatrhd self-assigned this Jul 14, 2022

rahmatrhd mentioned this issue Aug 4, 2022

feat(grant): import pre-existing access from provider #247

Merged

rahmatrhd mentioned this issue Aug 19, 2022

Unauthorised access tracking #173

Closed

ravisuhag closed this as completed in #247 Oct 7, 2022

rahmatrhd mentioned this issue Feb 15, 2023

Add API to assign/update owner of a grant #362

Closed

4 tasks

lifosmin pushed a commit to lifosmin/guardian that referenced this issue Aug 31, 2023

refactor!: drop ml prefix from types for feature, model (raystack#206)

0acbe8a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import pre-existing access from providers #206

Import pre-existing access from providers #206

Chief-Rishab commented Jul 7, 2022 •

edited by rahmatrhd

Loading

rahmatrhd commented Jul 7, 2022

ravisuhag commented Jul 7, 2022

ravisuhag commented Jul 7, 2022 •

edited

Loading

singhvikash11 commented Jul 8, 2022

rahmatrhd commented Jul 11, 2022

rahmatrhd commented Jul 29, 2022

Chief-Rishab commented Jul 29, 2022 •

edited

Loading

ravisuhag commented Jul 29, 2022

rahmatrhd commented Aug 4, 2022

rahmatrhd commented Aug 4, 2022

ravisuhag commented Aug 21, 2022

Import pre-existing access from providers #206

Import pre-existing access from providers #206

Comments

Chief-Rishab commented Jul 7, 2022 • edited by rahmatrhd Loading

Requirements

Approach:

1. Fetching access

2. Creating the appeals

Things to note

Policy

Account Types

Role

rahmatrhd commented Jul 7, 2022

ravisuhag commented Jul 7, 2022

ravisuhag commented Jul 7, 2022 • edited Loading

singhvikash11 commented Jul 8, 2022

rahmatrhd commented Jul 11, 2022

rahmatrhd commented Jul 29, 2022

Chief-Rishab commented Jul 29, 2022 • edited Loading

ravisuhag commented Jul 29, 2022

rahmatrhd commented Aug 4, 2022

rahmatrhd commented Aug 4, 2022

ravisuhag commented Aug 21, 2022

Chief-Rishab commented Jul 7, 2022 •

edited by rahmatrhd

Loading

ravisuhag commented Jul 7, 2022 •

edited

Loading

Chief-Rishab commented Jul 29, 2022 •

edited

Loading