Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish Authentication and Authorization #7615

Closed
1 of 6 tasks
aaronc opened this issue Oct 21, 2020 · 13 comments
Closed
1 of 6 tasks

Distinguish Authentication and Authorization #7615

aaronc opened this issue Oct 21, 2020 · 13 comments
Assignees
Labels
C:x/auth C:x/authz S:proposal accepted S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Proto Breaking Protobuf breaking changes: never don't do this!

Comments

@aaronc
Copy link
Member

aaronc commented Oct 21, 2020

Summary

Problem Definition

We will have two modules with the abbreviation auth in their name but which deal with two slightly different concerns.

x/auth deals primarily with "authentication". From Oxford authentication is

the process or action of verifying the identity of a user or process.

x/msg_authorization deals with what an "authenticated" user is "authorized" to do. From Oxford authorization is

a document giving permission or authority

Which seems pretty consistent with what x/msg_authorization does.

Proposal

There is a fair amount of precedent for using authn and authz as abbreviations for authentication and authorization respectively, as well as for auth alone being confusing:

So how about x/auth becomes x/authn and x/msg_authorization becomes x/authz?


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

ACK

aaronc added a commit that referenced this issue Oct 21, 2020
@clevinson
Copy link
Contributor

👍

@anilcse
Copy link
Collaborator

anilcse commented Oct 22, 2020

ACK

@aaronc aaronc added S:proposal accepted T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Proto Breaking Protobuf breaking changes: never don't do this! labels Oct 22, 2020
mergify bot pushed a commit that referenced this issue Jan 13, 2021
* Add ADR 030

* Cleanup

* Updates

* Updates based on ADR 031

* Add abstract and context

* Updates

* Update module name as per #7615

* Updates from reviews

* Rename

* CLI update

* Update docs/architecture/adr-030-authz-module.md

Co-authored-by: Robert Zaremba <[email protected]>

* Update docs/architecture/adr-030-authz-module.md

Co-authored-by: Robert Zaremba <[email protected]>

* Update docs/architecture/adr-030-authz-module.md

Co-authored-by: Robert Zaremba <[email protected]>

* Update docs/architecture/adr-030-authz-module.md

Co-authored-by: Robert Zaremba <[email protected]>

* Code review updates

* Update docs

Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Robert Zaremba <[email protected]>
@fedekunze
Copy link
Collaborator

fedekunze commented Jan 27, 2021

I don't understand why are we introducing such a breaking change. Couldn't we just name the authz module authorization instead? This is clearer as the authn/z abbreviations could be read as auth-n auth-z by non native english speakers.

@clevinson
Copy link
Contributor

I don't see an issue with reading as auth-n and auth-z. Is that really a problem? The important piece is to dimabiguate them from eachother.

We originally were just calling authz authorization, but having two modules (x/auth and x/authorization) is what was causing the initial confusion.

@fedekunze
Copy link
Collaborator

why don't we just rename authz to authorization? SDK devs are already familiar with the auth module and what it does... My take on this is that the breaking change is just not worth it

@aaronc
Copy link
Member Author

aaronc commented Feb 3, 2021

why don't we just rename authz to authorization? SDK devs are already familiar with the auth module and what it does... My take on this is that the breaking change is just not worth it

One (possibly bad) reason not to do this is because it makes protobuf type URLs longer - cosmos.authz vs cosmos.authorization

We could also have auth and authz and just not do the rename of auth -> authn

@fedekunze
Copy link
Collaborator

We could also have auth and authz and just not do the rename of auth -> authn

I don't have a strong opinion on the authz renaming, but I'm strongly against the auth -> authn renaming because it's such a large breaking change for other applications

@aaronc
Copy link
Member Author

aaronc commented Feb 4, 2021

@alexanderbez do you have any thoughts here? We moved forward because you ACK'ed but I would be fine keeping auth as is if there's strong opposition. @alessio ?

@alessio
Copy link
Contributor

alessio commented Feb 4, 2021

IMHO authn is a better name. I agree with @fedekunze though, I suspect the change wouldn't be very warmly welcomed by application developers. How about postponing the auth -> authn renaming to Cosmos SDK v1.0?

@aaronc
Copy link
Member Author

aaronc commented Feb 4, 2021

IMHO authn is a better name. I agree with @fedekunze though, I suspect the change wouldn't be very warmly welcomed by application developers. How about postponing the auth -> authn renaming to Cosmos SDK v1.0?

Totally fine to postpone it

@clevinson
Copy link
Contributor

Moving to icebox as this will ideally be addressed in the x /auth & x/bank refactor. Possibly with two modules in parallel (new & old auth / authz).

@aaronc
Copy link
Member Author

aaronc commented Jun 25, 2021

A proposed API for a new authn module is in #9369.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/auth C:x/authz S:proposal accepted S:proposed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). T: Proto Breaking Protobuf breaking changes: never don't do this!
Projects
None yet
Development

No branches or pull requests

8 participants