Added group member check

[renanqts]

#18

[k3a]

So if I understand it correctly, your use-case is not using Kubernetes with ClusterRole/ClusterRoleBinding but a pre-configured group-based check as an alternative implementation of Authorizer.

It would be mostly ok, but I don't like that it changes the API of Authorizer interface so much. Even the extra map allocation doesn't look very good to me. It would be ah-well-ok if it were an optional map with some extra metadata but not the main argument. Directly accessing map keys like requestVerb := m["requestVerb"].(string) can also easily produce panic if the map keys change without modifying other parts of codebase expecting those keys.

I think the better approach would be to keep the Authorizer api as is (user User, requestVerb, requestResource string) and use the first user argument to extract user groups for the purpose of that different authorizer.

Then, you would supply your configured authorizer in NewServer like so:

func NewServer(sessionStore sessions.Store, clientset kubernetes.Interface) *Server {
	s := &Server{
		log: internallog.NewDefaultLogger(config.LogLevel, config.LogFormat),
	}
	s.buildRoutes()
	s.sessionStore = sessionStore
	if config.EnableRBAC {
		s.authorizer = rbac.NewRBACAuthorizer(clientset)
	} else if config.GroupsMemberOf != "" {
		s.authorizer = groupmemberof.NewAuthorizer(config.GroupsMemberOf)
	}
	return s
}

That your authorizer would be just another implementation of the common Authorizer interface which is probably what we all would like. What do you think?

[jr0d]

Let's not change the Authorizer interface

[jr0d]

Suggested change

	GroupsMemberOf string `long:"groups-member-of" env:"GROUPS_MEMBER_OF" description:"List of groups that the user must be member, at least one"`
	GroupsMemberOf CommaSeparatedList `long:"groups-member-of" env:"GROUPS_MEMBER_OF" description:"List of groups that the user must be member, at least one"`

[renanqts]

That is okay for me.