Is your feature request related to a problem? Please describe.
The approvers in the provider files are at provider level and would be applicable to all resources for that provider.
But if we need an different approver in the same provider for one or two particular resources, we use the details column in the resources table to add specific approver for that resource only.

Whenever a new resource(dataset) gets added, the details is empty. and we have to manually populate the details column. and till the time details is populated, if an appeal is raised against such a resource - the guardian app panics and crashes.

panic: reflect: call of reflect.Value.Type on zero Value

goroutine 80 [running]:
reflect.Value.Type({0x0, 0x0, 0x133b74b})
  /opt/hostedtoolcache/go/1.17.7/x64/src/reflect/value.go:2262 +0x12e
github.com/odpf/guardian/core/appeal.(*Service).resolveApprovers(0xc0004b02d0, {0xc00053dfc0, 0x1, 0xc0013276c0}, 0xc001e6d9b0)
  /home/runner/work/guardian/guardian/core/appeal/service.go:500 +0x3e6
github.com/odpf/guardian/core/appeal.(*Service).fillApprovals(0xc0004b02d0, 0xc0005aaa00, 0xc000cc5860)

Describe the solution you'd like
Would like to have a way to override approvers in policy definition itself, in a way such as even if the details column is empty/nil. then the approvers are taken from the policy file itself.

Is your feature request related to a problem? Please describe.
Auto rejection approval is not supporting rejection message

Describe the solution you'd like
Need to have rejection_message key in the approval step config for auto approval

Additional context
message set in rejection_message will get passed to approval.reason when the approval get rejected automatically from the approve_if evaluation.
https://github.com/odpf/guardian/blob/main/domain/approval.go#L20

Summary
Guardian supports running jobs as commands. That means one should be able to run the jobs separately in a cron. I should be able to disable running jobs in server, so that I can run them separately outside the server in a cron.

This also resolves one other issue - Whenever guardian jobs run in the server, if we have multiple k8s pods - a user will receive multiple notifications at the same time from each pod. Running the jobs separately would ensure clean user experience and also separate out the cron jobs.

Proposed solution
Add enable/disable flag like below in server config

JOBS:
  FETCH_RESOURCES:
    Enable: True
    Interval: "0 */2 * * *"
  REVOKE_EXPIRED_ACCESS:
    Enable: True
    Interval: "*/20 * * * *"
  EXPIRING_ACCESS_NOTIFICATION:
    Enable: True
    Interval: "0 9 * * *"

Additional context
Guardian version >= v0.2.9

Describe the bug
Expiry reminders do not work for appeals created for a serviceAccount.

This snippet in AppealExpirationReminder method -

		for _, a := range appeals {
			notifications = append(notifications, domain.Notification{
				User: a.AccountID,
				Message: domain.NotificationMessage{
					Type: domain.NotificationTypeExpirationReminder,

should ideally be using appeal.CreatedBy instead of AccountID for user, as accountID would have serviceAccount and it would not have any slackID tied to it to send a notification to.

This issue doesn't exist when sending notification for revoke, since revoke flow uses appeal.AccountID for notifications.

Summary

In metabase, one can create collections inside collections, and there is no unique name constraint as metabase keeps track of collection id rather than name itself.

For example, I can have the following collections , where in I have the DS Analysis collection(different but same name) in under CabFares and also under Spending

# metabase collections
- Our Analytics
- Sample Collection
- CabFares
    - DS Analysis
        - Summary
- Spending
    - Countries
        - DS Analysis

If we add this metabase as a provider in existing guardian... when fetching resources (api endpoint: api/v1beta1/resources?provider_type=metabase), it returns the following response. In this, based on name - one cannot identify which collection it is - the one under CabFares or the one under Spending.
One can identify using the urn (collectionid), but it not understandable for a user when raising appeal which one to request for - unless they know the collectionId. Hence, the information is insufficient or unclear

{
  "resources": [
    {
      "id": "b1f0d1dc-afda-4e3f-8fcf-0f2167bf4aba",
      "provider_type": "metabase",
      "provider_urn": "odpf-metabase",
      "type": "collection",
      "urn": "collection:14",
      "name": "DS Analysis",
      "details": { ... },
      "created_at": "2022-06-23T08:00:07.035136Z",
      "updated_at": "2022-06-23T16:00:07.018758Z"
    },
    {
      "id": "98c4bf30-f021-49cc-927f-6e01ce5210ce",
      "provider_type": "metabase",
      "provider_urn": "odpf-metabase",
      "type": "collection",
      "urn": "collection:16",
      "name": "DS Analysis",
      "details": { ... },
      "created_at": "2022-06-06T08:37:38.681177Z",
      "updated_at": "2022-06-23T16:00:07.018758Z"
    }
    ...
  ]
}

Proposed solution
Metabase collections api - GET /api/collection/ also has the locations/hierarchy of these collections. Guardian can parse this, and store in the db, a collection full path name as - /CabFares/DS Analysis or /Spending/Countries/DS Analysis which would be a self sufficient value for a user to understand which collection it is when they are creating an appeal.

Is your feature request related to a problem? Please describe.
Guardian requires cli config to be present in binary path dir.

Describe the solution you'd like
Guardian should load CLI config from home dir or through an env var which can specify where to load the config from.

Requirements

• track activities related to data changes & audit logs

Proposed solution

Create a new table in db to store audit logs as well as create its repository and service. Audit Service will be used across other services to log the activity.

database model

type AuditLog struct {
	Timestamp  time.Time
	Action     string // example: appeal.created, provider.created, provider.updated, etc.
	Actor      string // example: [email protected] or system
	Data       interface{}
	Metadata   interface{}
}

implementation

1. repository
   implementation of this interface can be created in salt for reusability.

type AuditRepository interface {
	List(filters map[string]interface{}) ([]*AuditLog, error)
	Create(*AuditLog) error
	BulkCreate([]*AuditLog) error
}

2. guardian's audit service

type AuditAction string // typed audit action
var (
	ProviderCreated     AuditAction = "provider.created"
	ResourceBulkCreated AuditAction = "resource.bulkCreated"
	// ...
)

// typed audit data for each audit action
type ProviderCreatedData domain.Provider
type ResourceBulkCreatedData struct {
	CreatedResourceIDs []string
	RemovedResourceIDs []string
}

// audit service interface representation
type AuditService interface {
        Log(actor, action, message string, payload interface{}) error
}

// usage
auditService.Log("[email protected]", audit.ProviderCreated, "message example", audit.ProviderCreatedData{...})
auditService.Log("system", audit.ResourceBulkCreated, "message example", audit.ResourceBulkCreatedData{...})

guardian audit log list

To Reproduce

1. Create a new policy
2. Timestamps in the response will be 1970-01-01T00:00:00Z

Expected behavior
CreatedAt and UpdatedAt values should be the actual creating and updating time

Solution
keep passing nil/empty if GetCreatedAt() value is nil everywhere timestamppb.Timestamp is parsed in adapter.go

var createdAt time.Time
if xProto.GetCreatedAt() != nil {
    createdAt = xProto.GetCreatedAt().AsTime()
}

x.CreatedAt = createdAt

instead of the existing logic:
https://github.com/odpf/guardian/blob/f0f8f5b034fe26d790bf957ce7b833caf4e2e655/api/handler/v1beta1/adapter.go#L30-L31

Is your feature request related to a problem? Please describe.
When guardian fetches user information from the identity manager, it store all the returned fields into creator field in appeal. Need an ability to configure/map the returned fields into a customized creator object

Describe the solution you'd like
A clear and concise description of what you want to happen.
add fields mapping config in Policy.IAM config

Description
Extending an appeal that is linked to a policy with additional appeals was not working because guardian detected the existing additional appeals already exist.

We need to allow the appeal to be created (activated) even if the additional appeals are still active by skipping the additional appeals creation.

To Reproduce
Extending appeal that is already linked to a policy with additional appeal won't work

Expected behavior
Extending appeal that is already linked to a policy with additional appeal should work

Guardian package structure can be updated to allow for better readability and abstraction. The proposed structure is following.

store
  models
  ...
plugins
  iam
    shield
    http
  provider
    bigquery
    gcloud_iam
    ...
  notifier
    slack
core
  appeal
  provider
  resource
  policy
internal
  crypto
  scheduler
  evaluator

Describe the bug
The struct mapped to Metabase list database API response is invalid. It needs to be changed to:

type DatabaseResponse struct {
	Data []*Database `json:"data"`
}

To Reproduce
Steps to reproduce the behaviour:

1. Add Metabase providers with databases
2. See an error in the log

Expected behaviour
It should be able to parse databases and add these databases as resources in Guardian

Is your feature request related to a problem? Please describe.

1. No authentication methods supported yet
2. The existing HTTP provider has a pretty fixed contract which makes it hard to adapt to external services
   • response contract:
     https://github.com/odpf/guardian/blob/99ef4e3c06d1308ead17f9a9d9175bffe10a1631/iam/http.go#L16-L18
   • parameter:
     https://github.com/odpf/guardian/blob/99ef4e3c06d1308ead17f9a9d9175bffe10a1631/iam/http.go#L49-L51

Describe the solution you'd like

1. Add few authentications support
   • Basic
   • API Key
   • Bearer
2. Accept any response and allow user to configure field paths for getting the target value from the response
3. Configurable parameter passing
   • query
   • header
   • URL param

Describe the bug

"client not configured." error is printed even after the config init step.

❯ ./guardian config init
config created: /Users/sushmithbhatta/.config/odpf/guardian.yml
client not configured. try running `guardian config init`

To Reproduce
Steps to reproduce the behavior:

1. Remove ~/.config/odpf/guardian.yml file if exists in local
2. Run guardian config init after building.
3. See error in cli

Expected behavior

No error should be printed during the config init step if it runs successfully.

❯ ./guardian config init
config created: /Users/sushmithbhatta/.config/odpf/guardian.yml

Additional context
issue caused by defer - https://github.com/odpf/guardian/blob/main/main.go#L25

To Reproduce
Steps to reproduce the behavior:

1. set AUTHENTICATED_USER_HEADER_KEY=x-header-example-email
2. create an HTTP request with a x-header-example-emailor X-Header-Example-Email header
3. guardian will failed to read the header key
4. replace config to AUTHENTICATED_USER_HEADER_KEY=X-Header-Example-Email to make it works

Expected behavior
header should not be case sensitive

Describe the bug
Got some unexpected validation errors when creating policy using CLI:

1. Approvers min=1 validation error when strategy value is auto
2. unrecognized IAM provider type when not providing an IAM config

Expected behavior

1. strategy: auto expects no approvers to be present
2. IAM config is optional

For users who are maintaining policies through the version control system. Guardian can provide a plan and apply methods to see if there are any changes in the policies and update them.

Describe the bug
Consider an appeal is created which is tied to a policy containing only one auto approval step and the auto approval condition becomes true. If the provider config for that appeal allows permanent access, then the create method creates another record with an active status into the appeal table.

To Reproduce

• Create a policy definition with only one auto approval step and no manual ones.
• Tie up a provider with this policy and enable allow_permanent_access in the provider appeal config.
• Create an appeal
• Create an appeal again for the same resource and role with the same details (given that the appeal is within the allow_active_access_extension_in duration)
• There will be two active records in the table instead of only one.

Expected behavior
There should only be one active record in the table.

Additional context
This is happening because the logic to check if there is an already active appeal and to mark it as terminated is embedded within the MakeAction function call., which does not get triggered at all in the above case.

version
latest on main branch

Guardian can manage access across multiple providers. But it is still hard for data governance managers to monitor the different aspects of data access.

Goal:
With data access monitoring in Guardian, we aim to provide answers to the following questions.

• How many users have access to sensitive data?
• How many appeals are pending?
• How many appeals are about to expire?
• What kind of data authorized users are accessing?
• When was a resource accessed, by whom, and for what purpose?
• Answers to these questions are very important to be proactive in managing security and compliance.

Scope:
Access monitoring can be tracked across different sections

• Appeals: Analytics about appeals and their status.
• Access Logs: Analytics about what resources are being actually queried and how frequently.
• Users: Analytics about how many users are active, and have access across resources.
• Resources: Analytics about how many resources are available and of what type.

1. Fetch user information from external identity manager and put the details information in appeal
2. Allow identity config to map user details fields

1. Move identity manager config from app config to the policy
   https://github.com/odpf/guardian/blob/99ef4e3c06d1308ead17f9a9d9175bffe10a1631/iam/client.go#L14-L22
2. Read identity manager config from policy

Describe the bug
Guardian provides PUT API to update existing policy. But it's falling when the payload of policy has a version in it and that version is not the latest.

To Reproduce
Steps to reproduce the behaviour:

1. Create a policy
2. Update the policy with version 1
3. Update the policy with version 1

Expected behaviour
Either policy should be updated and create a new version instead of input-version, or it'll throw an error the policy is not the latest one so it'll not update.

REQUIREMENTS:

• Provider configuration for grafana

  1. Credentials config
  2. Permission config
  3. Documentation

• Setup grafana client

  1. Grafana HTTP Client
  2. Authentication

• Grafana resource & access management

  1. Get dashboard resources
  2. Grant access to dashboard
  3. Revoke access to dashboard

• Documentation

  1. Concept: Grafana implementation in guardian
  2. End-to-end guides to setup grafana

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

1. set up a postgres instance with empty password and host auth method as trust
2. pass the details to guardian and run migrate

Expected behavior
guardian should not panic and run fine.

Screenshots

Request for a feature to delete providers and its corresponding resources from guardian.

Currently, if a provider was added to guardian, then guardian automatically fetches the resources and populates the resources table. If in case, the provider and its resources should not be there in guardian at all and there are no appeals tied up with this provider, the only way to remove them would be to run a query on the guardian db directly since there are no api's implemented in guardian for removing providers.

some APIs expect an authorized user email to be present in the request header (metadata for GRPC). For example approve/reject approval API. On web, rely on the auth proxy to provide this header. But for guardian CLI we haven't had an auth mechanism for accessing those APIs.

Need to provide configurable authentication method in the CLI

Summary
To create an appeal guardian requires a provider to be registered. But in some cases, Guardian users might just want to take advantage of policy workflow and not load resources in Guardian. They might want to just use Guardian APIs for approval workflows and appeal management. This can also allow users to locally test Guardian easily without configuring an actual provider.

Proposed solution
Guardian can support a no-op provider which will require be placeholder provider with no resources. Users would be able to configure a policy on no-op providers and raise an appeal. In such cases, users would typically integrate with Guardian APIs and might want to manage resources for which appeals are being created on the application side itself.

Add support for tableau access:

• - Introduce provider configuration for tableau
• - Init tableau client & authentication method
• - Get Workbook resources
• - Grant & Revoke access to Workbooks
• - Get Flow resources
• - Grant Revoke access for the Flow
• - Get View resources
• - Grant Revoke access for the View
• - Get Metric resources
• - Grant Revoke access for the Metric
• - Get Data source resources
• - Grant Revoke access for the Data Source

Migrate to using config package from salt in Guardian.
https://github.com/odpf/salt/config

Is your feature request related to a problem? Please describe.
In some places, user experience can be improved to show better messages.

Describe the solution you'd like

• Add init command for a policy and provider
• Print better blank state errors - no resource found.
• Show spinner when fetching results in commands (with tty)

Describe the bug
A clear and concise description of what the bug is.

1. on provider create/update, allowed_account_types field is ignored and not being passed to db
2. empty allowed_account_types failed to use default value automatically

We have moved out api handler to api/handler package, and repositories was also moved to store, which left each domain package with the service implementation and job the handlers. Job handlers should be treated the same way as API handlers because from architectural point of view, what both are doing is: receiving inputs -> call services -> and return response (basically handler).

expected structure :

  ...
  api
    handler
      grpc.go
  core
    appeal 
      service.go
      service_test.go
-     job_handlers.go
    provider
      service.go
      service_test.go
-     job_handlers.go
+ jobs
+   appeal_expiration_reminder.go
+   appeal_expiration_revoke.go
+   refresh_resources.go
  ...

That way we make our code cleaner 🙌

additional:

right now we're still running the jobs from the server, ideally we should expose each job, and let the user decide how the job is triggered. In that case, we can create a CLI command to expose the jobs such as:

$ guardian jobs {{job_name}} run

Feature request:

An API to fetch different provider types registered/present in guardian.

Reason:

Currently, when guardian api is integrated with a UI to enable & onboard the users for appeal creation, with the api's provided by guardian, all the resources of all providers would be available at once for the user to filter out and select what he/she would want to appeal for.

With a single kind of provider registered, this doesn't pose much of a problem. But with different kinds of additional providers registered like - bigquery, metabase, tableau, etc., it gets confusing super quick from user perspective to select.

With this API, the UI would be able to give options to user what type of provider access they are requesting for at a very high level. and in turn filtering the resources to show only the particular provider ones.

Add a new provider for GCP IAM which allows users to give any GCP IAM role to a user.

Describe the solution you'd like

1. accept dry_run flag in payload

  message CreatePolicyRequest {
      Policy policy = 1;
+     bool dry_run = 2;
  }

  message UpdatePolicyRequest {
      string id = 1;
      Policy policy = 2;
+     bool dry_run = 3;
  }

  message CreateProviderRequest {
      ProviderConfig config = 1;
+     bool dry_run = 2;
  }

  message UpdateProviderRequest {
      string id = 1;
      ProviderConfig config = 2;
+     bool dry_run = 2;
  }

2. add support to policy service and provider service to accept dry_run info from context

// core/policy/service.go

func WithDryRun(ctx context.Context) context.Context {
  return context.WithValue(ctx, "dry_run")
}

func isDryRun(ctx) bool {
  return ctx.Value("dry_run").(bool)
}

...

func (s *Service) Create(ctx context.Context, p *domain.Policy) error {
  ...

  dryRun := isDryRun(ctx)
  if !dryRun {
    if err := s.policyRepository.Create(p); err != nil {
		return err
	}
  }

  return nil
}

3. pass dry_run flag to service in grpc handler

// api/handler/v1beta1/grpc.go GRPCServer.CreatePolicy
...

if req.GetDryRun() {
  ctx = policy.WithDryRun(ctx)
}

if err := s.policyService.Create(ctx, policy); err != nil {
  return nil, status.Errorf(codes.Internal, "failed to create policy: %v", err)
}

...

Problem?
Guardian approval flow with policies makes sure only approved users get access to resources. But if someone was authorized to access directly through the provider platform e.g users are given access to Metabase through its admin portal. Guardian does not have a way to track if access is given to the provider resources which are not being tracked or approved through guardian access workflows.

Solution

• Periodically check authorized users at the provider level and track unauthorized access to resources.
• Provide reports and alerts when unauthorized access is detected.
• Raised alert severity can be based on the privacy level of resources.

Right now we're still running the jobs from the server, ideally we should expose each job, and let the user decide how the job is triggered. In that case, we can create a CLI command to expose the jobs such as:

$ guardian jobs {{job_name}} run

for this PR: #162

Describe the bug
custom groups created by guardian in metabase are added as resources

Context

1. existing metabase provider supports collection and database resource access
2. (in metabase) database, table, and collection access are granted to groups, and a group has many users
3. while in guardian, a user access is pointing to a resource directly.
   1. so to accommodate that, we make the metabase provider create a group that is specific for accessing a resource with a specific role, in other words, this group will always have access to one resource with one role.
   2. see below diagram for illustration
      
4. we're currently introducing 2 new resources in guardian which are table (child resource under database) and group
5. table resource is fine, but for group, we don't want to include those custom groups as resources in guardian

Expected behavior
custom groups shouldn't be added as resources in guardian

Proposed Solution

1. have a naming convention for the group name e.g. add prefix _guardian_<group_name>
2. when fetching group resources from metabase, exclude any groups that have prefix _guardian_

Is your feature request related to a problem? Please describe.

1. Need to allow users to create an appeal for a service account

Describe the solution you'd like

1. Introduce allowed account types in the provider configuration

  type: bigquery
  urn: pilotdata-integration
  credentials: ...
+ allowed_account_types:
+ - user
+ - serviceAccount
  Resources: ...

• Supported account types are different for each provider
  • Bigquery: user, serviceAccount
  • Gcloud IAM: user, serviceAccount
  • ...
• allowed_account_types is optional and the default value is all supported account types for that provider.

2. Introduce account type in appeal

--- a/domain/appeal.go
+++ b/domain/appeal.go
@@ -30,7 +30,9 @@ type Appeal struct {
        PolicyID      string                 `json:"policy_id"`
        PolicyVersion uint                   `json:"policy_version"`
        Status        string                 `json:"status"`
-       User          string                 `json:"user"`
+       AccountID     string                 `json:"account_id"`
+       AccountType   string                 `json:"account_type" default:"user"`
+       CreatedBy     string                 `json:"created_by"`
        Role          string                 `json:"role"`
        Options       *AppealOptions         `json:"options"`
        Details       map[string]interface{} `json:"details"`

• Add created_by field to store the requester or authenticated user email
• Rename user field to account_id
• Add optional account_type field with default value: “user”

3. Fetch User details based on the requestor
   • #69

missing features in CLI:

• account_type flag in appeals create command
• command for showing the approval statuses (visualization)
• resource get metadata cmd
• appeal revoke
• appeal cancel

Use salt logger for logging in Guardian.

https://github.com/odpf/salt/log

having interface for a service implies that there could be multiple implementation of services, which shouldn't happen because service is handling business logic, and there should be only one place for defining business logic. So service interfaces shouldn't be there, and use the service struct directly.

Summary
At the moment, for every provider one can configure a set of roles with permission sets. It enables one to control the access roles to which a user can raise appeals for.

Bigquery example - https://odpf.github.io/guardian/docs/providers/bigquery#yaml-representation which has roles and permission set for each resource type.

This is not true for gcloud_iam provider. Only this provider does not have a way to take in the configured roles/permissions from the provider config file. Due to which if the gcloud_iam is exposed as a provider to users, the GetRoles method will fetch every iam role available in GCP. If one wants to restrict users to only a certain roles through guardian like other providers, they cannot currently., and the user would be able to raise an appeal for any role in gcp.

Proposed solution
Modify the gcp provider to

• get roles from the provider config if roles are provided, and use them as the only roles available for appeals.
• continue as it is in the current form if the roles are not provided in the provider config.

Describe the bug

id: auto_approval
steps:
- name: auto_approval
  strategy: auto
  approve_if: 'true'

This auto approval policy which the steps are ended with one strategy: auto step, won't automatically resolve the appeal's status on appeal creation

Expected behavior
With that policy, the appeal should be automatically marked as approved on appeal creation

Context

Once a provider is registered, guardian will fetch all the supported resource types from the provider, even though the provider configuration only specifying for some resource types.

Example:

# provider config
type: bigquery
resources:
- type: dataset
  policy: ...
  roles: ...
  ...
...

With this config, guardian not only will fetch all datasets, but also all tables.
This could cause an issue if a user is creating an appeal to a table resource from that provider because the policy and roles are not configured yet for that table resource type.

Proposed solution

Make guardian to only fetch the specified resource types based on the provider config.

Description
We send notifications - e.g: appeal expiry, the function call Notify(items []domain.Notification) error is called with all the notifications to be sent. But whenever there is an error for any given user, the function call returns back without trying to send notifications to the other users in the list.

func call -

func (n *notifier) Notify(items []domain.Notification) error {
	for _, item := range items {
		slackID, err := n.findSlackIDByEmail(item.User)
		if err != nil {
			return err
		}

		msg, err := parseMessage(item.Message, n.Messages)
		if err != nil {
			return err
		}

		if err := n.sendMessage(slackID, msg); err != nil {
			return err
		}
	}

	return nil
}

This should ideally catch all the errors and return them all at once in the end, instead of in the middle.

Additional context
Whenever a user leaves the org in slack, slack throws the error users_not_found when trying to fetch slackID using email.