diff --git a/storage/kubernetes/client.go b/storage/kubernetes/client.go
index 04ee9cae28afb007d3f3bcc381edc0fc12282b0e..dfb1fe35688f1b95c3231749df6ceee5ea847947 100644
--- a/storage/kubernetes/client.go
+++ b/storage/kubernetes/client.go
@@ -4,10 +4,13 @@ import (
"bytes"
"crypto/tls"
"crypto/x509"
+ "encoding/base32"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
+ "hash"
+ "hash/fnv"
"io"
"io/ioutil"
"net"
@@ -31,6 +34,14 @@ type client struct {
baseURL string
namespace string
+ // Hash function to map IDs (which could span a large range) to Kubernetes names.
+ // While this is not currently upgradable, it could be in the future.
+ //
+ // The default hash is a non-cryptographic hash, because cryptographic hashes
+ // always produce sums too long to fit into a Kubernetes name. Because of this,
+ // gets, updates, and deletes are _always_ checked for collisions.
+ hash func() hash.Hash
+
// API version of the oidc resources. For example "oidc.coreos.com". This is
// currently not configurable, but could be in the future.
apiVersion string
@@ -40,6 +51,18 @@ type client struct {
cancel context.CancelFunc
}
+// idToName maps an arbitrary ID, such as an email or client ID to a Kubernetes object name.
+func (c *client) idToName(s string) string {
+ return idToName(s, c.hash)
+}
+
+// Kubernetes names must match the regexp '[a-z0-9]([-a-z0-9]*[a-z0-9])?'.
+var encoding = base32.NewEncoding("abcdefghijklmnopqrstuvwxyz234567")
+
+func idToName(s string, h func() hash.Hash) string {
+ return strings.TrimRight(encoding.EncodeToString(h().Sum([]byte(s))), "=")
+}
+
func (c *client) urlFor(apiVersion, namespace, resource, name string) string {
basePath := "apis/"
if apiVersion == "v1" {
@@ -277,6 +300,7 @@ func newClient(cluster k8sapi.Cluster, user k8sapi.AuthInfo, namespace string) (
return &client{
client: &http.Client{Transport: t},
baseURL: cluster.Server,
+ hash: func() hash.Hash { return fnv.New64() },
namespace: namespace,
apiVersion: "oidc.coreos.com/v1",
}, nil
diff --git a/storage/kubernetes/client_test.go b/storage/kubernetes/client_test.go
index 92ae204a5b7c2356f97c7ce73fea6e7fda9048cf..01b4eb7a9b25f567c8f60c05b54e11183dea5f77 100644
--- a/storage/kubernetes/client_test.go
+++ b/storage/kubernetes/client_test.go
@@ -1,6 +1,33 @@
package kubernetes
-import "testing"
+import (
+ "hash"
+ "hash/fnv"
+ "sync"
+ "testing"
+)
+
+// This test does not have an explicit error condition but is used
+// with the race detector to detect the safety of idToName.
+func TestIDToName(t *testing.T) {
+ n := 100
+ var wg sync.WaitGroup
+ wg.Add(n)
+ c := make(chan struct{})
+
+ h := func() hash.Hash { return fnv.New64() }
+
+ for i := 0; i < n; i++ {
+ go func() {
+ <-c
+ name := idToName("foo", h)
+ _ = name
+ wg.Done()
+ }()
+ }
+ close(c)
+ wg.Wait()
+}
func TestNamespaceFromServiceAccountJWT(t *testing.T) {
namespace, err := namespaceFromServiceAccountJWT(serviceAccountToken)
diff --git a/storage/kubernetes/storage.go b/storage/kubernetes/storage.go
index bd48b3db39788eece493e91c0853715e32807fe4..05e088d489544dbf771e421691085012323608a3 100644
--- a/storage/kubernetes/storage.go
+++ b/storage/kubernetes/storage.go
@@ -5,6 +5,7 @@ import (
"fmt"
"log"
"net/http"
+ "strings"
"time"
"golang.org/x/net/context"
@@ -187,21 +188,47 @@ func (cli *client) GetAuthCode(id string) (storage.AuthCode, error) {
}
func (cli *client) GetClient(id string) (storage.Client, error) {
- var c Client
- if err := cli.get(resourceClient, id, &c); err != nil {
+ c, err := cli.getClient(id)
+ if err != nil {
return storage.Client{}, err
}
return toStorageClient(c), nil
}
+func (cli *client) getClient(id string) (Client, error) {
+ var c Client
+ name := cli.idToName(id)
+ if err := cli.get(resourceClient, name, &c); err != nil {
+ return Client{}, err
+ }
+ if c.ID != id {
+ return Client{}, fmt.Errorf("get client: ID %q mapped to client with ID %q", id, c.ID)
+ }
+ return c, nil
+}
+
func (cli *client) GetPassword(email string) (storage.Password, error) {
- var p Password
- if err := cli.get(resourcePassword, emailToID(email), &p); err != nil {
+ p, err := cli.getPassword(email)
+ if err != nil {
return storage.Password{}, err
}
return toStoragePassword(p), nil
}
+func (cli *client) getPassword(email string) (Password, error) {
+ // TODO(ericchiang): Figure out whose job it is to lowercase emails.
+ email = strings.ToLower(email)
+ var p Password
+ name := cli.idToName(email)
+ if err := cli.get(resourcePassword, name, &p); err != nil {
+ return Password{}, err
+ }
+ if email != p.Email {
+ return Password{}, fmt.Errorf("get email: email %q mapped to password with email %q", email, p.Email)
+ }
+ return p, nil
+}
+
func (cli *client) GetKeys() (storage.Keys, error) {
var keys Keys
if err := cli.get(resourceKeys, keysName, &keys); err != nil {
@@ -242,7 +269,12 @@ func (cli *client) DeleteAuthCode(code string) error {
}
func (cli *client) DeleteClient(id string) error {
- return cli.delete(resourceClient, id)
+ // Check for hash collition.
+ c, err := cli.getClient(id)
+ if err != nil {
+ return err
+ }
+ return cli.delete(resourceClient, c.ObjectMeta.Name)
}
func (cli *client) DeleteRefresh(id string) error {
@@ -250,28 +282,34 @@ func (cli *client) DeleteRefresh(id string) error {
}
func (cli *client) DeletePassword(email string) error {
- return cli.delete(resourcePassword, emailToID(email))
+ // Check for hash collition.
+ p, err := cli.getPassword(email)
+ if err != nil {
+ return err
+ }
+ return cli.delete(resourcePassword, p.ObjectMeta.Name)
}
func (cli *client) UpdateClient(id string, updater func(old storage.Client) (storage.Client, error)) error {
- var c Client
- if err := cli.get(resourceClient, id, &c); err != nil {
+ c, err := cli.getClient(id)
+ if err != nil {
return err
}
+
updated, err := updater(toStorageClient(c))
if err != nil {
return err
}
+ updated.ID = c.ID
newClient := cli.fromStorageClient(updated)
newClient.ObjectMeta = c.ObjectMeta
- return cli.put(resourceClient, id, newClient)
+ return cli.put(resourceClient, c.ObjectMeta.Name, newClient)
}
func (cli *client) UpdatePassword(email string, updater func(old storage.Password) (storage.Password, error)) error {
- id := emailToID(email)
- var p Password
- if err := cli.get(resourcePassword, id, &p); err != nil {
+ p, err := cli.getPassword(email)
+ if err != nil {
return err
}
@@ -279,10 +317,11 @@ func (cli *client) UpdatePassword(email string, updater func(old storage.Passwor
if err != nil {
return err
}
+ updated.Email = p.Email
newPassword := cli.fromStoragePassword(updated)
newPassword.ObjectMeta = p.ObjectMeta
- return cli.put(resourcePassword, id, newPassword)
+ return cli.put(resourcePassword, p.ObjectMeta.Name, newPassword)
}
func (cli *client) UpdateKeys(updater func(old storage.Keys) (storage.Keys, error)) error {
diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go
index 8c8f9d6eee91072d4d9c99b83bdfdbbcd6ffe07a..900a8187b2be0023d4b236ff259545116b57c9e9 100644
--- a/storage/kubernetes/types.go
+++ b/storage/kubernetes/types.go
@@ -1,7 +1,6 @@
package kubernetes
import (
- "encoding/base32"
"strings"
"time"
@@ -75,13 +74,14 @@ const keysName = "openid-connect-keys"
// Client is a mirrored struct from storage with JSON struct tags and
// Kubernetes type metadata.
-//
-// TODO(ericchiang): Kubernetes has an extremely restricted set of characters it can use for IDs.
-// Consider base32ing client IDs.
type Client struct {
+ // Name is a hash of the ID.
k8sapi.TypeMeta `json:",inline"`
k8sapi.ObjectMeta `json:"metadata,omitempty"`
+ // ID is immutable, since it's a primary key and should not be changed.
+ ID string `json:"id,omitempty"`
+
Secret string `json:"secret,omitempty"`
RedirectURIs []string `json:"redirectURIs,omitempty"`
TrustedPeers []string `json:"trustedPeers,omitempty"`
@@ -106,9 +106,10 @@ func (cli *client) fromStorageClient(c storage.Client) Client {
APIVersion: cli.apiVersion,
},
ObjectMeta: k8sapi.ObjectMeta{
- Name: c.ID,
+ Name: cli.idToName(c.ID),
Namespace: cli.namespace,
},
+ ID: c.ID,
Secret: c.Secret,
RedirectURIs: c.RedirectURIs,
TrustedPeers: c.TrustedPeers,
@@ -120,7 +121,7 @@ func (cli *client) fromStorageClient(c storage.Client) Client {
func toStorageClient(c Client) storage.Client {
return storage.Client{
- ID: c.ObjectMeta.Name,
+ ID: c.ID,
Secret: c.Secret,
RedirectURIs: c.RedirectURIs,
TrustedPeers: c.TrustedPeers,
@@ -258,17 +259,6 @@ type Password struct {
UserID string `json:"userID,omitempty"`
}
-// Kubernetes only allows lower case letters for names.
-//
-// NOTE(ericchiang): This is currently copied from the storage package's NewID()
-// method. Once we refactor those into the storage, just use that instead.
-var encoding = base32.NewEncoding("abcdefghijklmnopqrstuvwxyz234567")
-
-// Map an arbitrary email to a valid Kuberntes name.
-func emailToID(email string) string {
- return strings.TrimRight(encoding.EncodeToString([]byte(strings.ToLower(email))), "=")
-}
-
func (cli *client) fromStoragePassword(p storage.Password) Password {
email := strings.ToLower(p.Email)
return Password{
@@ -277,7 +267,7 @@ func (cli *client) fromStoragePassword(p storage.Password) Password {
APIVersion: cli.apiVersion,
},
ObjectMeta: k8sapi.ObjectMeta{
- Name: emailToID(email),
+ Name: cli.idToName(email),
Namespace: cli.namespace,
},
Email: email,