Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions pkg/auth/oauth/external/handler.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package external

import (
"encoding/base64"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -147,11 +148,11 @@ func (defaultState) Generate(w http.ResponseWriter, req *http.Request) (string,
"csrf": {"..."}, // TODO: get csrf
"then": {req.URL.String()},
}
return state.Encode(), nil
return encodeState(state)
}

func (defaultState) Check(state string, w http.ResponseWriter, req *http.Request) (bool, error) {
values, err := url.ParseQuery(state)
values, err := decodeState(state)
if err != nil {
return false, err
}
Expand All @@ -169,7 +170,7 @@ func (defaultState) Check(state string, w http.ResponseWriter, req *http.Request
}

func (defaultState) AuthenticationSucceeded(user user.Info, state string, w http.ResponseWriter, req *http.Request) (bool, error) {
values, err := url.ParseQuery(state)
values, err := decodeState(state)
if err != nil {
return false, err
}
Expand All @@ -182,3 +183,15 @@ func (defaultState) AuthenticationSucceeded(user user.Info, state string, w http
http.Redirect(w, req, then, http.StatusFound)
return true, nil
}

func encodeState(values url.Values) (string, error) {
return base64.URLEncoding.EncodeToString([]byte(values.Encode())), nil
}

func decodeState(state string) (url.Values, error) {
decodedState, err := base64.URLEncoding.DecodeString(state)
if err != nil {
return nil, err
}
return url.ParseQuery(string(decodedState))
}
147 changes: 147 additions & 0 deletions pkg/auth/oauth/external/keycloak/keycloak.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package keycloak

import (
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/url"
"strings"

"github.com/RangelReale/osincli"
"github.com/golang/glog"

authapi "github.com/openshift/origin/pkg/auth/api"
"github.com/openshift/origin/pkg/auth/oauth/external"
)

type provider struct {
Realm string `json:"realm"`
RealmPublicKey string `json:"realm-public-key"`
ClientID string `json:"resource"`
Credentials struct {
Secret string `json:"secret"`
} `json:"credentials"`
AuthServerURL string `json:"auth-server-url"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the keycloak config have a way to specify the trusted roots of the auth server? That wasn't an issue for github/google, but it likely will need to be able to be specified when hitting a self-hosted auth server. The Provider interface would need a new GetTransport() method which could return nil by default, and be overridden if trusted roots needed to be set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also means that if the provider config will include a file reference for a trusted cert bundle, NewProviderFromConfigData would need a path to resolve file references relative to

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The documentation doesn't cover any of this. I suspect this is left up to the user to ensure they've configured their key/trust stores appropriately. I'll walk through their ssl example and see what shakes out. I should probably also test ssl on the OS side of things as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I incorporated the other feedback, but was not able to get an https connection working. Looking at the code, it appears the auth transport is configured with the root certs for the OS instance. I was able to confirm that the cert used by the KC application was being picked up and included in that list and while I was able to get past some authorization errors (e.g. no SAN IPs), I was not able to get past "certificate signed by unknown authority." This was even when I used the OS CA cert to sign the cert used by KC. This is pretty far outsided my realm of expertise and it's possible I just haven't created the certificate properly (e.g. the SAN IPs issue above). Regardless, the change should be good to go, barring the fact that you can't use https for the KC auth server at this time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the outgoing request to the external OAuth server actually uses the system trusted roots, so I'm not surprised it's not picking up the OpenShift certs

}

type keycloakUser struct {
id string
username string
email string
firstName string
lastName string
realmRoles string
}

func NewProviderFromFile(keycloakClientConfigFile string) (external.Provider, error) {
keycloakClientConfigBytes, err := ioutil.ReadFile(keycloakClientConfigFile)
if err != nil {
glog.Errorf("Error loading Keycloak config: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if an error is a possibility, return multiple values: the provider, and the error. don't make a nil return value signal an error.

return nil, err
}
return NewProviderFromBytes(keycloakClientConfigBytes)
}

func NewProviderFromBytes(keycloakClientConfigBytes []byte) (external.Provider, error) {
p := provider{}
err := json.Unmarshal(keycloakClientConfigBytes, &p)
if err != nil {
glog.Errorf("Error parsing Keycloak config: %s", err)
return nil, err
}
return p, nil
}

// NewConfig implements external/interfaces/Provider.NewConfig
func (p provider) NewConfig() (*osincli.ClientConfig, error) {
realmURL, err := url.Parse(p.AuthServerURL)
if err != nil {
return nil, err
}
realmURL.Path += "/realms/" + p.Realm + "/"
loginPath := url.URL{
Path: "tokens/login",
}
tokenPath := url.URL{
Path: "tokens/access/codes",
}
config := &osincli.ClientConfig{
ClientId: p.ClientID,
ClientSecret: p.Credentials.Secret,
ErrorsInStatusCode: true,
SendClientSecretInParams: true,
AuthorizeUrl: realmURL.ResolveReference(&loginPath).String(),
TokenUrl: realmURL.ResolveReference(&tokenPath).String(),
}
return config, nil
}

// AddCustomParameters implements external/interfaces/Provider.AddCustomParameters
func (p provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
}

// GetUserIdentity implements external/interfaces/Provider.GetUserIdentity
func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentityInfo, bool, error) {
idToken, ok := data.ResponseData["id_token"].(string)
if !ok {
return nil, false, fmt.Errorf("No id_token returned in %v", data.ResponseData)
}

userdata, err := decodeJWT(idToken)
glog.V(4).Infof("userdata=%v", userdata)
if err != nil {
return nil, false, err
}

username, _ := userdata["preferred_username"].(string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this mutable (I don't know the keycloak schema)? we want to make sure the value returned from the identity server is one we can count on not changing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a keycloak expert either, but this field appears to map to the username in keycloak, which cannot be changed. To be honest, much of this was reverse engineered based on what was coming back from the keycloak server. I don't know the source well and I couldn't find any detailed API documentation.

if username == "" {
return nil, false, errors.New("Could not retrieve Keycloak username")
}

identity := &authapi.DefaultUserIdentityInfo{
UserName: username,
Extra: map[string]string{
"name": userdata["name"].(string),
"email": userdata["email"].(string),
},
}
glog.V(4).Infof("identity=%v", identity)

return identity, true, nil
}

// Decode JWT
// http://openid.net/specs/draft-jones-json-web-token-07.html
func decodeJWT(jwt string) (map[string]interface{}, error) {
jwtParts := strings.Split(jwt, ".")
if len(jwtParts) != 3 {
return nil, fmt.Errorf("Invalid JSON Web Token: expected 3 parts, got %d", len(jwtParts))
}

encodedPayload := jwtParts[1]
glog.V(4).Infof("got encodedPayload")

// Re-pad, if needed
if l := len(encodedPayload) % 4; l != 0 {
padding := strings.Repeat("=", 4-l)
encodedPayload += padding
glog.V(4).Infof("added padding: %s\n", padding)
}

decodedPayload, err := base64.StdEncoding.DecodeString(encodedPayload)
if err != nil {
return nil, fmt.Errorf("Error decoding payload: %v\n", err)
}
glog.V(4).Infof("got decodedPayload")

var data map[string]interface{}
err = json.Unmarshal([]byte(decodedPayload), &data)
if err != nil {
return nil, fmt.Errorf("Error parsing token: %v\n", err)
}
glog.V(4).Infof("got id_token data")

return data, nil
}
12 changes: 12 additions & 0 deletions pkg/auth/oauth/external/keycloak/keycloak_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package keycloak

import (
"testing"

"github.com/openshift/origin/pkg/auth/oauth/external"
)

func TestKeycloak(t *testing.T) {
provider, _ := NewProviderFromBytes([]byte(""))
_ = external.Provider(provider)
}
13 changes: 11 additions & 2 deletions pkg/cmd/server/origin/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/openshift/origin/pkg/auth/oauth/external"
"github.com/openshift/origin/pkg/auth/oauth/external/github"
"github.com/openshift/origin/pkg/auth/oauth/external/google"
"github.com/openshift/origin/pkg/auth/oauth/external/keycloak"
"github.com/openshift/origin/pkg/auth/oauth/handlers"
"github.com/openshift/origin/pkg/auth/oauth/registry"
authnregistry "github.com/openshift/origin/pkg/auth/oauth/registry"
Expand Down Expand Up @@ -108,6 +109,8 @@ const (
AuthHandlerGithub AuthHandlerType = "github"
// AuthHandlerGoogle redirects unauthenticated requests to Google to request an OAuth token.
AuthHandlerGoogle AuthHandlerType = "google"
// AuthHandlerKeycloak redirects unauthenticated requests to Keycloak to request an OAuth token.
AuthHandlerKeycloak AuthHandlerType = "keycloak"
// AuthHandlerDeny treats unauthenticated requests as failures
AuthHandlerDeny AuthHandlerType = "deny"
)
Expand Down Expand Up @@ -376,7 +379,7 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
var authHandler handlers.AuthenticationHandler
authHandlerType := c.AuthHandler
switch authHandlerType {
case AuthHandlerGithub, AuthHandlerGoogle:
case AuthHandlerGithub, AuthHandlerGoogle, AuthHandlerKeycloak:
callbackPath := path.Join(OpenShiftOAuthCallbackPrefix, string(authHandlerType))
userRegistry := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy())
identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(string(authHandlerType) /*for now*/, userRegistry)
Expand All @@ -386,6 +389,12 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand
oauthProvider = google.NewProvider(c.GoogleClientID, c.GoogleClientSecret)
} else if authHandlerType == AuthHandlerGithub {
oauthProvider = github.NewProvider(c.GithubClientID, c.GithubClientSecret)
} else if authHandlerType == AuthHandlerKeycloak {
var err error
oauthProvider, err = keycloak.NewProviderFromFile(c.KeycloakClientConfigFile)
if err != nil {
return nil, fmt.Errorf("unexpected error creating KeyCloak OAuth provider: %v", err)
}
}

state := external.DefaultState()
Expand Down Expand Up @@ -468,7 +477,7 @@ func (c *AuthConfig) getAuthenticationSuccessHandler() handlers.AuthenticationSu
}

switch c.AuthHandler {
case AuthHandlerGithub, AuthHandlerGoogle:
case AuthHandlerGithub, AuthHandlerGoogle, AuthHandlerKeycloak:
successHandlers = append(successHandlers, external.DefaultState().(handlers.AuthenticationSuccessHandler))
case AuthHandlerLogin:
successHandlers = append(successHandlers, redirectSuccessHandler{})
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/origin/auth_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ type AuthConfig struct {
GithubClientID string
// GithubClientID is the client_secret of a client registered with the GitHub OAuth provider.
GithubClientSecret string

// KeycloakClientConfigFile is the json file used to configure the Keycloak OAuth provider.
// This file can be generated using the contents from the "Installation" tab for the OAuth client in the Keycloak admin console.
KeycloakClientConfigFile string
}

func BuildAuthConfig(options configapi.MasterConfig) (*AuthConfig, error) {
Expand Down Expand Up @@ -137,6 +141,8 @@ func BuildAuthConfig(options configapi.MasterConfig) (*AuthConfig, error) {
// GitHub config
GithubClientID: cmdutil.Env("OPENSHIFT_OAUTH_GITHUB_CLIENT_ID", ""),
GithubClientSecret: cmdutil.Env("OPENSHIFT_OAUTH_GITHUB_CLIENT_SECRET", ""),
// Keycloak config
KeycloakClientConfigFile: cmdutil.Env("OPENSHIFT_OAUTH_KEYCLOAK_CLIENT_CONFIG_FILE", ""),
}

return ret, nil
Expand Down