Summary

ODPF project structure standard prefer to follow domain-driven structure where each domain is located inside its domain main package inside core package.

Although code in shield is currently tidy enough, there is still a model and structs package holds all entities and services in internal/{package}. With the approach like this, it is more likely for the developers to not separate the concern well in the further development considering there is no restriction when adding a functionality for a domain. This can also lead to more complex spaghetti code that mixed logic between several domains.

We need to follow ODPF project structure standard slowly to have a design restriction so we can be more mindful when developing a new functionalities. I am also hoping this will simplify the code base and increase the readability & predictability.

2 points need to be focused on

• restructure project
• restructure middleware (userid responsibilities)

Proposed solution

We can divide this into multiple tasks covering two points mentioned above. Some changes needed are (each changes could be in its own PR).

restructure project

Move all domain to core/{domain} package ⛓️ PR #123 Merged

• Currently it is unclear what are domains in shield, we can start by moving all entities in model to each of its own domain
• Moving services in internal/{package} to its own domain in core/{domain}
• Fatal should only happened in cmd level, don't use Fatal inside the service/repository, propagate error to cmd level instead

Refactor server and store ⛓️ PR #130 Merged

• Move store and server inside internal/server & internal/store
• All initialization should be inside cmd package and dependencies are injected there
• There are also possibilities to have internal/middleware, internal/proxy and internal/hook since it is only being used by server (moving them to pkg does make sense as well but there is no point to expose it IMO)
• structs and utils packages can be refactored to be more readable
• v1beta1 handler needs to be moved to internal/api
• resources_config and rules config could be just centralized inside config package
• Some variables named with Id better to rename it to ID

Breakdown postgres store into multiple domain repositories ⛓️ PR #131 In Progress

• Currently store/postgres does not use repository but blob and local has one, therefore store/postgres has no separation of domain concern, the store does all responsibilities. We need to separate the responsibility by introducing domain repository and move all functions in store/postgres to its own doman repository.
• Add unit test with dockertest

There should ideally no logic changes with the tasks above. There is a remaining focus to restructure the middleware Middleware restructuring (userid responsibilities) that can be analyzed post project restructure.

clean up middleware ⚙️ PR #135

• Cleaning up how we propagate rules, request body, path params via context in proxy server. Context setter and getter is defined in its domain package e.g. rule is in rule packae, for request body and path params we create a new package in pkg/httputil
• Service and handler layer should not directly scanning and consuming header value from grpc metadata. Only grpc interceptor and proxy middleware that should does that and pass the value inside context. Service and handler layer only checking the value passed in context

Describe the bug Server is still running fine if spicedb is not running and resources & rule config are 0. But when a call to h2c proxy is executed, there is an error to connect to spicedb.

To Reproduce Steps to reproduce the behavior:

1. Resource and rules folder should be empty thus making resource_config and rule_config to 0.
2. Make sure spicedb is not running
3. Run shield server
4. Server is running

Expected behavior Server should catch and return error early when spice db is not running during the server initiation even though the resource_config and rule_config are 0. There is a need to do ping/light query check to spicedb when server is initiated.

We use Dockerfile.dev to build a tag dev. Our current Dockerfile.dev copied shield to /usr/local/bin/shield but the Dockerfile is copying shield to usr/bin/shield.

We expect the same path between Dockerfile and Dockerfile.dev. We need to update Dockerfile.dev so shield is copied to /usr/bin/shield.

Changes

• Copy shield to usr/bin
• Add glibc for compatibility purpose

Describe the bug Creating namespace via API with body contains only name (the id was not passed). This will create a row in table namespaces and return response with empty id.

To Reproduce Steps to reproduce the behavior: 1.

curl --location --request POST 'http://localhost:8000/admin/v1beta1/namespaces' \
--header 'X-Shield-Email: [email protected]' \
--header 'Content-Type: application/json' \
--data-raw '{
    "name": "req ns"
}'

2. Response got

{
    "namespace": {
        "id": "",
        "name": "req ns",
        "createdAt": "2022-07-22T06:02:34.396844Z",
        "updatedAt": "2022-07-22T06:02:34.396844Z"
    }
}

3. Row in namespaces table

Expected behavior ID in table row should not be empty

Request:

GET shield.io/admin/v1beta1/objects/{id}/relations?subject_kind=user&relation_type=member

Response:

{
    "relations": [
        {
            "user": {
                // shall have the entire user object
            },
            "group": {
                // shall remain empty as the type is user
            },
            "type": "user",
            "role": "member"
        },
        {
            "user": {
                // shall remain empty as the type is group
            },
            "group": {
                // shall have the entire group object
            },
            "type": "group",
            "role": "owner"
        }
    ]
}

Add utility to do end-to-end test

• Spin up 2 Postgres and 1 Spice DB in such a way that they are connected
• All are being done programmatically with dockertest
• Once all DBs are up, run shield migration and shield server with cobra cli

• Add mockery for mocking interface
• For APIs that require Auth Email header, return 403 Forbidden if not present/user does not exist
• Fix error type naming
• Create a function to check empty string
• Add tests in handler

Status code changes:

• Bad request -> 400
• Conflict/Insert existing resource -> 409
• Unauthorized (in check permission) -> 403 (not 401 since that is used when not able to establish identity from credentials or credentials are missing)
• No identity header -> 401
• Server related error -> 500

List Groups of a User api is giving us 500 Error

GET /admin/v1beta1/users/682f6492-f6ca-43ec-91ec-48ffad05b38e/groups

{
    "code": 13,
    "message": "internal server error",
    "details": []
}

This is due to role_id being made complusary - internal/store/postgres/group_repository.go#L461

	if roleID == "" || userID == "" {
		return nil, group.ErrInvalidID
	}

	query, params, err := dialect.Select(
		goqu.I("g.id").As("id"),
		goqu.I("g.metadata").As("metadata"),
		goqu.I("g.name").As("name"),
		goqu.I("g.slug").As("slug"),
		goqu.I("g.updated_at").As("updated_at"),
		goqu.I("g.created_at").As("created_at"),
		goqu.I("g.org_id").As("org_id"),
	).From(goqu.L("relations r")).
		Join(goqu.L("groups g"), goqu.On(
			goqu.I("g.id").Cast("VARCHAR").
				Eq(goqu.I("r.object_id")),
		)).Where(goqu.Ex{
		"r.object_namespace_id": namespace.DefinitionTeam.ID,
		"subject_namespace_id":  namespace.DefinitionUser.ID,
		"subject_id":            userID,
		"role_id":               roleID,
	}).ToSQL()

This is not the behaviour expected as we need to return all the groups member if role_id was empty

With respect to changes in principals for Resources, we are supposed to change PG schema which is very fixed with Team and Owner. We will move to only Owner role for resource and it can be either of principals

Tasks on Resources:

• Remove owner & team
• Addition of new principals on owner

Currently, our schema only lets us configure backend and resource, like odin_dagger, but not entities like org, project & team

example we cannot change this schema from any dynamic config file, but from code

definition organization {
	relation organization_admin: user | team#team_member
	permission manage_organization = organization_admin
	permission create_project = organization_admin
	permission create_team = organization_admin
}

definition project {
	relation organization: organization
	relation project_admin: user | team#team_member
	permission manage_project = organization->organization_admin + project_admin
}

definition team {
	relation organization: organization
	relation project: project
	relation team_admin: user
	relation team_member: user
	permission all_actions_team = project->project_admin + organization->organization_admin
	permission manage_team = organization->organization_admin + team_admin
	permission view_team = team_member + team_admin
}

definition user {}

if say we need to add a new relation in org, we need to make changes in code.

we are seeing that for upcoming changes being planned under new schema changes , we would need to make certain changes in entity definitions as well.

Thus, moving to a more flexible configuration yaml

organization:
    type: entity
    roles: 
        - name: experimenter
          type: 
            - user
            - team#member
    capabilities:
        - name: litmus_experiment_view
          roles:
            - owner
            - editor

odin:
    type: backend
    resources:
        - name: firehose
            roles: 
            - name: owner
            type: 
                - user
                - root#super_admin

Tasks

• [x] Error handling when cli config not found, for api that does not need config, could still do the call, otherwise return warning
• [x] Loading config for client init, pick from the root, but the flag could override it
• [x] Loading protos from proton based on a commit in Makefile
• [x] Namespacing server commands
• [x] Init for server config

Error handling in client config The existence of $ODPF_CONFIG_DIR/shield.yaml

• If user does not have this file in the path
  • calling shield client cli without --host would throw error need a config or flag --host
  • calling shield client cli that not requires auth
    • with --host would pass
  • calling shield cli that requires auth
    • with --host would return --header is required (we mark this as required)
    • with --host and --header would pass

Some changes in cli

+ shield config init // initializing client config
+ shield config list // list down client configurations
+ shield server init // initializing server config

- shield serve
+ shield server start

- shield migrate
+ shield server migrate

- shield migrate-rollback
+ shield server migrate-rollback

Describe the bug Resource updation query currently needs resource urn and resource user_id, which are not passed to the repository layer and are not even required.

To Reproduce Try making a resource update request

Expected behavior Resource shall be successfully updated without these two column values.

Current State: We only have authz check for Resource Group Namespaces

Expected State: We should also have authz check layer for System Namespaces as well

We need APIs on how to assign roles to System & Resource Group Namespaces

Tasks

1. RPC(s) on Proton for Role Assignment
2. Role creation (creating namespace and principal relation) on SpiceDB
3. Role creation on ShieldDB

Acceptance Criteria: User should be able to assign role to any principal, for any namespace

There are many places where we are using both namespace.Namespace and NamespaceID. We had planned to only use IDs instead of using whole object

type Action struct {
	ID          string
	Name        string
	NamespaceID string
        Namespace   namespace.Namespace     // <- Depreciate this
	CreatedAt   time.Time
	UpdatedAt   time.Time
}

This should be depreciated from codebase as well as proto