bcc-code / developer.bcc.no

BCC Developer Portal with technical documentation and other resources for developers
https://developer.bcc.no/
5 stars 5 forks source link

[ADR] Auth0 scope naming convention #45

Closed JakubC-projects closed 2 years ago

JakubC-projects commented 2 years ago

Context

Propositions

Decision

<Entity>[.<SubEntity>]#<Permission>

For parsing we use

For reading we use (snakecase)

Scopes and permissions are defined and configured by Terraform and shouldn't be maintained manually.

Consequences

See conversation below.

Alternatives

See conversation below.

github-actions[bot] commented 2 years ago

Remember that ADRs are publicly available hence do not include any confidential information in the issue description! To read more about ADR please refer to documentation.

rvanoord commented 2 years ago

Suggest making (optional) allowance for tenants as part of the scope. A tenant could be represented by "tenant-name" such as "bccoslofollo", "poland", "bcctonsberg" and represents an organization or group of organizations that control the data (GDPR datacontroller)

u12206050 commented 2 years ago

Alternative based on Google's Zanzibar permission system is: <NAMESPACE>:[<TENANT>/]DOCUMENT>#<VERB>

eg: persons:412#manage with sub fields persons:412.email#view You can even do persons:*#view or persons:*.display#view

Support for tenants forms part of the document

eg. persons:bccoslofollo/*#view or we can decide to use orgId persons:69/412#manage

rvanoord commented 2 years ago

Perhaps scoping down to document level is a bit too fine grained for OAuth scopes?

u12206050 commented 2 years ago

The flexibility of it means you can stop at the collection level, whilst still supporting individual fields, at the same time have it the option of scoping at document level if the need ever arises.

u12206050 commented 2 years ago

So for Auth0 this would probably be enough persons:*.email#view persons:*.profile#view persons:*.profile#manage

or with limited to tenants persons:bccoslofollow/*.email#view persons:bccoslofollow/*.profile#view persons:bccoslofollow/*.profile#manage

piotrczyz commented 2 years ago

In one way proposed approach by @u12206050 gives a lot of power and flexibility. I'm just a tiny little bit worried that this is too much overkill for our use cases. I don't see a use case where the same admin will have access to a document with ID 123 but not to a document with an ID 124.

In our case, if an admin/person has access to do something, he can do it for all users in the application or within a tenant.

I agree with Reng's proposal.

andreasgangso commented 2 years ago

I just want to clarify something here. This is for access tokens, right? So do I understand it correctly:

If this is correct then gerards convention is best because it allows you to actually limit on the document level if you ever need to. But you don't have to implement the document level in code if you are never going to use it.

u12206050 commented 2 years ago

That is correct Andreas. Although I doubt we would create a separate scope for representing each user. Although if Auth0 supports some sort of hook or logic that scopes can be generated that it could work.

As stated previously, this format can be both as corse as you want persons:*#manage or as fine grained as you want persons:oslofollo/12345.profile#view, which in my opinion means we can use it for both the format of scopes and of permissions.

I like how you phrase it "the oauth scopes are only used to limit the permissions to less than the user might have in another context." which means the scopes can just be a simple label representing the permission ie persons#view or persons#manage

u12206050 commented 2 years ago

Does it mean though that just because you then can request a scope via Auth0 that you still don't get the relevant permissions in the service, if those permissions are not activated for the role you have? Or as an application don't you have a role?

rvanoord commented 2 years ago

In general, I think of "scopes" as static, course-grained permissions which can be managed at a cross-team, organizational level. E.g. "which APIs should this client be allowed to access?" or "This organization can have access to reading the events API, scoped to their tenant data".

Fine grained, document level permissions are often based on dynamic rules. and certainly relate to dynamic data (e.g. document 123 doesn't exist anywhere in code). This is not the sort of permissions that "scopes" are intended to solve. However, that's not to say we couldn't have a centralized permisssion service (like Zanzibar - which I take to be a distributed permissions cache) - but that's just a slightly different topic.

@andreasgan @u12206050

philipdalen commented 2 years ago

Suggest making (optional) allowance for tenants as part of the scope. A tenant could be represented by "tenant-name" such as "bccoslofollo", "poland", "bcctonsberg" and represents an organization or group of organizations that control the data (GDPR datacontroller)

I don't agree with this. I feel it would make scopes too dynamic, in your example we could have 3 email:read scopes.

Will it not grow out of control quickly? Eventually we could end up with hundreds if not thousands of scopes

On the other hand if we would say, me as a 3rd party application belongs to tenant X, then if I get the email:read scope it is implicitely scoped to tenant X.

On the other hand we already agreed to the structure in this Teams group I see no ready why we need to re-consider this just be because we moved the conversation to a github issue. To me the Teams discussion was an ADR, it just wasn't in Github.

In summary this is what was agreed on by everybody then.

I think however we could clarify system_name to entity_name, if everybody agrees?

JakubC-projects commented 2 years ago

I think however we could clarify system_name to entity_name, if everybody agrees?

Agree

Also then some example scopes would look like this (right ?):

u12206050 commented 2 years ago

What was discussed in Teams was probably a good decision at that point in time, however since new information has come to light :)

I agree scopes should not be as fined grained as permissions, and neither should they be tenanted. The tenant can either be determined from the Client Credentials (identity) or explicitly defined HTTP header TenentID

When it comes to the actual syntax I still think be able to be as corse/fine grained as possible is very beneficial and separating parts with a unique symbol makes it easy to understand what one scope limits on or not.

service[:path][.field]#ability

persons:1234.email#read // read person email with 1234 persons:1234#read // read person with id 1234 persons.display_name#read // read display_name field on persons persons#read // read all persons

if needed it can still support tenanted scopes service[/tenant][:path][.field]#ability

persons/oslo#read // read all persons in the oslo tenant

philipdalen commented 2 years ago

@u12206050 I like the structure of your scope, meaning For Parsing we use "." to determine scope levels. Scopes alwasy goes from big (nonambiguous) to small (eg. BrunstadTV.searchhistory#read, or person.address.firstline#write) "#" to prefix ability

Reading we use (snakecase)

The think I don't see as being practical is :path, if this would be included it would generate thousands of scopes, which is not maintainable in my opinion

Examples Special scopes

Person Service App Scopes

Membership Service App Scopes

Brunstad TV

BMM

Roles & Scopes Service

u12206050 commented 2 years ago

The :path is just to show that if need be in the one or two services that need it we can be as specific as a path. That doesn't mean we will use it for most cases.

On Wed, 18 May 2022, 10:01 Philip, @.***> wrote:

@u12206050 https://github.com/u12206050 I like the structure of your scope, meaning For Parsing we use "." to prefix one or more fields "#" to prefix ability "first word" will always be the entity.

Reading we use "_" simply to make things more readable

The think I don't see as being practical is :path, if this would be included it would generate thousands of scopes, which is not maintainable in my opinion

— Reply to this email directly, view it on GitHub https://github.com/bcc-code/bcc-code.github.io/issues/45#issuecomment-1129695531, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVMWVNNBS7AOTAHXV6FYTVKSPUHANCNFSM5ULDTD5Q . You are receiving this because you were mentioned.Message ID: @.***>

u12206050 commented 2 years ago

Looks good. Agree we don't need tenant in the scopes as it is the tenant itself that requests the scopes and gets consents for it.

github-actions[bot] commented 2 years ago

The document will be available under following link

u12206050 commented 2 years ago

Whilst implementing this I realised the need for a namespace when listing multiple scopes from different applications. So would like to append to the convention namespace/…