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

[FEATURE REQUEST]get username from http basic auth and other types of auth #63

Open
tuhao1020 opened this issue Sep 26, 2020 · 21 comments

Comments

@tuhao1020
Copy link

https://github.com/iris-contrib/middleware/blob/master/casbin/casbin.go#L72

this line of code get username only by BasicAuth
In production environment, there will be other types of auth, such as session, token, jwt, OAuth...

@AlbinoGeek
Copy link

@tuhao1020 It's difficult to believe that iris would be aware of your specific setup of the database, among other things. It is not a framework like Meteor for example, which has a builtin session and user authentication to the point where it forces you to code the way their developer dictates.

That being said, if you are using mvc, you could make a Username field available using Dependency Injection.

Please see:

And more specifically:

@tuhao1020
Copy link
Author

@AlbinoGeek Is it possible to provide a callback interface so that users can customize the method to get the user name, role or other informations

@kataras
Copy link
Member

kataras commented Oct 9, 2020

@tuhao1020 you mean in the builtin and iris-contrib middlewares? You can use the ctx.Values() wrap every auth middleware you use and set the proper values.

I believe this is not necessary as we don't have much of auth middlewares controlled by us. There are hundreds of go packages for that job, I think it's not possible and productive to find all of them and "export" their roles and username, it's like designing a whole new package without any realistic benefits (my opinion). However, if you give me a list of middlewares that you use and need that type of feature, I will see what I can do.

@tuhao1020
Copy link
Author

@kataras considering that I use a simple string token encoded with Base64, and username can be decoded from it. (this is just an example to show you the usage) if I use this middleware, I need to copy all the code and rewrite the builtin function func (c *Casbin) Check(r *http.Request) bool and func Username(r *http.Request) string. Any other way to do this better?

kataras added a commit that referenced this issue Oct 9, 2020
@kataras
Copy link
Member

kataras commented Oct 9, 2020

OK @tuhao1020,

  • The SetUsername function was added

Note that you could modify the username before too, using the ctx.Request().SetBasicAuth("username", "password")*

  • The Check(*http.Request) bool method was replaced with Check(iris.Context) bool
  • The Wrapper method was removed entirely, use Party#UseRouter(cm.ServeHTTP) instead

Further feedback is welcomed!

@tuhao1020
Copy link
Author

@kataras
got an error:

github.com/iris-contrib/middleware/casbin: github.com/iris-contrib/middleware/[email protected]: parsing go.mod: go.mod:7:2: require github.com/kataras/iris/v12: version "master" invalid: must be of the form v1.2.3

@kataras
Copy link
Member

kataras commented Oct 10, 2020

Wait, go get does not resolve the master by itself? Why go build does?

Anyway, it's fixed now, please try again.

@tuhao1020
Copy link
Author

@kataras OK! it works.

@kataras
Copy link
Member

kataras commented Oct 12, 2020

@tuhao1020 I though of this yesterday and added a Context.Logout and a Context.SetLogoutFunc to "logout" from basicauth but these functions can be used by any middleware to add a custom logout implementation.

I'm also thinking of your proposal, as a more generic view, e.g. to have a Context.User() which will return a User interface (e.g. (User {Username(), ...} ) which can be customized by middleware(s), e.g. sessions, basicauth, casbin and e.t.c.

Sounds good?

@tuhao1020
Copy link
Author

@kataras Sounds fantastic! 👍

@kataras
Copy link
Member

kataras commented Oct 12, 2020

Hello @tuhao1020, so far the implementation of that User feature is simple, and the basic auth middleware supports it:

// SetUser sets a User for this request.
// It's used by auth middlewares as a common
// method to provide user information to the
// next handlers in the chain.
Context.SetUser(u User)

// User returns the registered User of this request.
// See `SetUser` too.
Context.User() User

The User interface looks like this:

package context

import (
	"errors"
	"time"
)

// ErrNotSupported is fired when a specific method is not implemented
// or not supported entirely.
// Can be used by User implementations when
// an authentication system does not implement a specific, but required,
// method of the User interface.
var ErrNotSupported = errors.New("not supported")

// User is a generic view of an authorized client.
// See `Context.User` and `SetUser` methods for more.
//
// The informational methods starts with a "Get" prefix
// in order to allow the implementation to contain exported
// fields such as `Username` so they can be JSON encoded when necessary.
//
// The caller is free to cast this with the implementation directly
// when special features are offered by the authorization system.
type User interface {
	// GetAuthorization should return the authorization method,
	// e.g. Basic Authentication.
	GetAuthorization() string
	// GetAuthorizedAt should return the exact time the
	// client has been authorized for the "first" time.
	GetAuthorizedAt() time.Time
	// GetUsername should return the name of the User.
	GetUsername() string
	// GetPassword should return the encoded or raw password
	// (depends on the implementation) of the User.
	GetPassword() string
	// GetEmail should return the e-mail of the User.
	GetEmail() string
}

// FeaturedUser optional interface that a User can implement.
type FeaturedUser interface {
	User
	// GetFeatures should optionally return a list of features
	// the User implementation offers.
	GetFeatures() []UserFeature
}

// UserFeature a type which represents a user's optional feature.
// See `HasUserFeature` function for more.
type UserFeature uint32

// The list of standard UserFeatures.
const (
	AuthorizedAtFeature UserFeature = iota
	UsernameFeature
	PasswordFeature
	EmailFeature
)

// HasUserFeature reports whether the "u" User
// implements a specific "feature" User Feature.
//
// It returns ErrNotSupported if a user does not implement
// the FeaturedUser interface.
func HasUserFeature(user User, feature UserFeature) (bool, error) {
	if u, ok := user.(FeaturedUser); ok {
		for _, f := range u.GetFeatures() {
			if f == feature {
				return true, nil
			}
		}

		return false, nil
	}

	return false, ErrNotSupported
}

// SimpleUser is a simple implementation of the User interface.
type SimpleUser struct {
	Authorization string        `json:"authorization"`
	AuthorizedAt  time.Time     `json:"authorized_at"`
	Username      string        `json:"username"`
	Password      string        `json:"-"`
	Email         string        `json:"email,omitempty"`
	Features      []UserFeature `json:"-"`
}

var _ User = (*SimpleUser)(nil)

// GetAuthorization returns the authorization method,
// e.g. Basic Authentication.
func (u *SimpleUser) GetAuthorization() string {
	return u.Authorization
}

// GetAuthorizedAt returns the exact time the
// client has been authorized for the "first" time.
func (u *SimpleUser) GetAuthorizedAt() time.Time {
	return u.AuthorizedAt
}

// GetUsername returns the name of the User.
func (u *SimpleUser) GetUsername() string {
	return u.Username
}

// GetPassword returns the raw password of the User.
func (u *SimpleUser) GetPassword() string {
	return u.Password
}

// GetEmail returns the e-mail of the User.
func (u *SimpleUser) GetEmail() string {
	return u.Email
}

// GetFeatures returns a list of features
// this User implementation offers.
func (u *SimpleUser) GetFeatures() []UserFeature {
	if u.Features != nil {
		return u.Features
	}

	var features []UserFeature

	if !u.AuthorizedAt.IsZero() {
		features = append(features, AuthorizedAtFeature)
	}

	if u.Username != "" {
		features = append(features, UsernameFeature)
	}

	if u.Password != "" {
		features = append(features, PasswordFeature)
	}

	if u.Email != "" {
		features = append(features, EmailFeature)
	}

	return features
}

kataras added a commit to kataras/iris that referenced this issue Oct 12, 2020
@AlbinoGeek
Copy link

You are well on your way to competing with more robust frameworks by implementing this, very interesting indeed!

@tuhao1020
Copy link
Author

type SimpleUser struct {
	Authorization string        `json:"authorization"`
	AuthorizedAt  time.Time     `json:"authorized_at"`
	Username      string        `json:"username"`
	Password      string        `json:"-"`
	Email         string        `json:"email,omitempty"`
	Features      []UserFeature `json:"-"`
}

I suggest that the property Email string change to AddtionalData map[string]interface{}

@AlbinoGeek
Copy link

AlbinoGeek commented Oct 13, 2020

@tuhao1020 Ehh; and here's why "ehh":

SimpleUser is an example implementation of User interface. You can (and should) completely replace this struct in your own implementation. The reason to include Email string is demonstrated pretty well, I feel, with the EmailFeature shown in GetFeatures()

@tuhao1020
Copy link
Author

@AlbinoGeek SimpleUser is in package context,it is a default implemention for User interface,change one property can meet the needs of many people. Out of the box is much better than customization, isn't it?

@AlbinoGeek
Copy link

AlbinoGeek commented Oct 13, 2020

Honestly I would disagree, and that's simply because Golang is a language of interfaces, and he has given a system of Features which allows you to add much more specific additional data directly into your own type MyUser struct however you may define it. Having an AdditionalData section would only serve to muddy up the ultimate use of the struct, such as when serializing it to JSON or the database -- not all databases can handle nesting well (for example.)

I would say it is significantly more common a case that an accounts system is based on email addresses and not usernames in a modern web application. Even more so, those that use OAuth exclusively as the means of providing authentication -- these technically don't user the Username or Email fields directly, but an internal AccountID related to the OAuth Connection being used -- which, more often than not, has an Email but not a Username

Examples of services without a concept of a static Username but with an Email:

  • Battle.Net
  • Discord
  • Facebook
  • Google
  • Microsoft (365, Teams, etc.)
  • Steam

I think it's best to err on the side of consistency, with compatibility through overrides as a second runner.


It is increasingly common nowadays for a Username to be outright replaced with a combination of Display Name and Vanity URL -- such is the case with Facebook, LinkedIn, Steam, Twitter, etc. -- And, I would argue that neither of these pieces of data belong in the User struct in your database, but in the Profile instead -- as the User should be a minimal record that can easily be cached, and yet has enough information to join/reference all other required tables/structs (such as the Profile).

@kataras
Copy link
Member

kataras commented Oct 14, 2020

Wow I am glad this is going interesting. I am working the last 2 days to provide a jwt user (always based on the context.User interface) implementation too (plus some features).

@tuhao1020 I thought about it a lot before Ι came up with that simple User interface solution instead of a map or adding an AditionalData field there because the ctx.User() returns an interface, which, as @AlbinoGeek said correctly, if you want any additional data you can cast it to the implementation directly so you have typed access to the fields. Another idea is to have your own type map which will implement the context.User (yes it's possible with Go) and then you can have any fields you want but they will not be type-safe(<--- this is the most important part on why a map is not a good idea generally) and that's why we have the UserFeature type which contain the basic feautes, e.g. EmailFeature, RolesFeature, UsernameFeature, TokenFeature (you can add your own too as long as not conflict with the builtins) that you can check with HasUserFeature before accessing a User's method (some more methods are added to the User interface, you will see them on the upcoming commit).

We must keep the User interface as simple as possible while useful on the majority of the cases and also power developer with the freedom of extending based on the application's requirements. The idea is to have the base context.User and provide implementations which will extend that based on auth middlewares. For example at the new JWT features I'm designing, so far, I was able to create simple NewUser method and VerifyUser middleware and simply accessing the verified client through ctx.User() without further steps from the end-developer. The ctx.User() can also be casted to a *jwt.User which may contain more fields (I believe is not necessary for the 99% of the cases).

As always, it's still under development, except basic authentication the context.User interface is not used anywhere else so far, so we can discuss it further. E.g. change the Username to DisplayName (however that will be confused for basic auth).

I am impressed from your comments, exactly the thoughts and the decisions I had while implementing that. Good job guys!

@kataras
Copy link
Member

kataras commented Oct 14, 2020

@tuhao1020, a GetField(key string) (interface{},error) and FieldsRole were added to the User interface too. It may be useful for dynamic options but I do strongly recommend to use a custom user implementation if you have large amount of custom fields in order to be safe and easier to use while your app is extending.

@kataras
Copy link
Member

kataras commented Oct 15, 2020

News, so far, except the new NewUser, VerifyUser and ctx.GetUser():

  • The JWT builtin middleware will support raw map[string]interface{} too, same features as static go structs with an embedded Claims type.

  • A new jwt.WithExpected will be available to set further claims protection against validation of issuer, subject and id

On the Context User feature:

@tuhao1020, your feature request is respected with the GetField (see my previous comment) but there is also a new builtin implementation of the User interface; UserMap which you can pass any map[string]interface{} to convert it as a compatible User, e.g. ctx.SetUser(UserMap{"username": "value", ...}). Also a trick, if you don't want to implement all User's methods, you can just embed the User in your Go structure for example, if you only want to implement the GetUsername you do that:

type MyUser struct {
  iris.User
  Username string
  CustomField string
}

func (u *MyUser) GetUsername() string {
  return u.Username
}

// STORE
ctx.SetUser(&MyUser{Username: "myusername", CustomField: "custom field"})

// RETRIEVE
username := ctx.User().GetUsername()
// ...
customField := ctx.User().(*MyUser).CustomField

@tuhao1020
Copy link
Author

@kataras Thanks! I really appreciate your work. Don't worry, I just only have two or three custom fields at most.

@kataras
Copy link
Member

kataras commented Oct 16, 2020

@tuhao1020 I am still working on that all with the JWT, you will be suprised of the new features!! Sleepless for 40 hours

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants