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

oauth2: Adds JWT Access Token strategy #947

Merged
merged 4 commits into from
Jul 23, 2018
Merged
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
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ jobs:
- run: go install github.com/ory/hydra/test/mock-lcp
- run: go-acc -o coverage.txt ./...
- run: go test -race -short $(go list ./... | grep -v cmd)
- run: ./scripts/test-e2e.sh
- run: ./scripts/test-e2e-jwt.sh
- run: ./scripts/test-e2e-opaque.sh
- run: test -z "$CIRCLE_PR_NUMBER" && goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls"

swagger:
Expand Down
5 changes: 1 addition & 4 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ type Client struct {
// ClientID is the id for this client.
ClientID string `json:"client_id"`

// ID is an alisa for client_id.
ID string `json:"id"`

// Name is the human-readable string name of the client to be presented to the
// end-user during authorization.
Name string `json:"client_name"`
Expand Down Expand Up @@ -145,7 +142,7 @@ type Client struct {
}

func (c *Client) GetID() string {
return c.ID
return c.ClientID
}

func (c *Client) GetRedirectURIs() []string {
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ fosite.Client = new(Client)

func TestClient(t *testing.T) {
c := &Client{
ID: "foo",
ClientID: "foo",
RedirectURIs: []string{"foo"},
Scope: "foo bar",
TokenEndpointAuthMethod: "none",
Expand Down
1 change: 0 additions & 1 deletion client/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request, ps httprouter.P
secret = c.Secret
}

c.ID = ps.ByName("id")
c.ClientID = ps.ByName("id")
if err := h.Validator.Validate(&c); err != nil {
h.H.WriteError(w, r, err)
Expand Down
8 changes: 7 additions & 1 deletion client/manager_0_sql_migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package client_test
import (
"fmt"
"log"
"sync"
"testing"

"github.com/jmoiron/sqlx"
Expand Down Expand Up @@ -116,6 +117,7 @@ var migrations = map[string]*migrate.MemoryMigrationSource{
}

func TestMigrations(t *testing.T) {
var m sync.Mutex
var dbs = map[string]*sqlx.DB{}
if testing.Short() {
return
Expand All @@ -127,14 +129,18 @@ func TestMigrations(t *testing.T) {
if err != nil {
log.Fatalf("Could not connect to database: %v", err)
}
m.Lock()
dbs["postgres"] = db
m.Unlock()
},
func() {
db, err := dockertest.ConnectToTestMySQL()
if err != nil {
log.Fatalf("Could not connect to database: %v", err)
}
m.Lock()
dbs["mysql"] = db
m.Unlock()
},
})

Expand All @@ -154,7 +160,7 @@ func TestMigrations(t *testing.T) {
s := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
c, err := s.GetConcreteClient(key)
require.NoError(t, err)
assert.EqualValues(t, c.ID, key)
assert.EqualValues(t, c.GetID(), key)
})
}

Expand Down
15 changes: 6 additions & 9 deletions client/manager_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ func (m *MemoryManager) GetConcreteClient(id string) (*Client, error) {
defer m.RUnlock()

for _, c := range m.Clients {
if c.ID == id {
c.ClientID = c.ID
if c.GetID() == id {
return &c, nil
}
}
Expand All @@ -67,7 +66,7 @@ func (m *MemoryManager) GetClient(_ context.Context, id string) (fosite.Client,
}

func (m *MemoryManager) UpdateClient(c *Client) error {
o, err := m.GetClient(context.Background(), c.ID)
o, err := m.GetClient(context.Background(), c.GetID())
if err != nil {
return err
}
Expand All @@ -88,7 +87,7 @@ func (m *MemoryManager) UpdateClient(c *Client) error {
m.Lock()
defer m.Unlock()
for k, f := range m.Clients {
if f.GetID() == c.ID {
if f.GetID() == c.GetID() {
m.Clients[k] = *c
}
}
Expand All @@ -109,13 +108,12 @@ func (m *MemoryManager) Authenticate(id string, secret []byte) (*Client, error)
return nil, errors.WithStack(err)
}

c.ClientID = c.ID
return c, nil
}

func (m *MemoryManager) CreateClient(c *Client) error {
if _, err := m.GetConcreteClient(c.ID); err == nil {
return errors.Errorf("Client %s already exists", c.ID)
if _, err := m.GetConcreteClient(c.GetID()); err == nil {
return errors.Errorf("Client %s already exists", c.GetID())
}

m.Lock()
Expand Down Expand Up @@ -152,8 +150,7 @@ func (m *MemoryManager) GetClients(limit, offset int) (clients map[string]Client

start, end := pagination.Index(limit, offset, len(m.Clients))
for _, c := range m.Clients[start:end] {
c.ClientID = c.ID
clients[c.ID] = c
clients[c.GetID()] = c
}

return clients, nil
Expand Down
5 changes: 2 additions & 3 deletions client/manager_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func sqlDataFromClient(d *Client) (*sqlData, error) {
}

return &sqlData{
ID: d.ID,
ID: d.GetID(),
Name: d.Name,
Secret: d.Secret,
RedirectURIs: strings.Join(d.RedirectURIs, "|"),
Expand All @@ -239,7 +239,6 @@ func sqlDataFromClient(d *Client) (*sqlData, error) {

func (d *sqlData) ToClient() (*Client, error) {
c := &Client{
ID: d.ID,
ClientID: d.ID,
Name: d.Name,
Secret: d.Secret,
Expand Down Expand Up @@ -301,7 +300,7 @@ func (m *SQLManager) GetClient(_ context.Context, id string) (fosite.Client, err
}

func (m *SQLManager) UpdateClient(c *Client) error {
o, err := m.GetClient(context.Background(), c.ID)
o, err := m.GetClient(context.Background(), c.GetID())
if err != nil {
return errors.WithStack(err)
}
Expand Down
6 changes: 6 additions & 0 deletions client/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"flag"
"fmt"
"log"
"sync"
"testing"

_ "github.com/go-sql-driver/mysql"
Expand All @@ -35,6 +36,7 @@ import (
)

var clientManagers = map[string]Manager{}
var m sync.Mutex

func init() {
clientManagers["memory"] = NewMemoryManager(&fosite.BCrypt{})
Expand All @@ -61,7 +63,9 @@ func connectToMySQL() {
}

s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
m.Lock()
clientManagers["mysql"] = s
m.Unlock()
}

func connectToPG() {
Expand All @@ -71,7 +75,9 @@ func connectToPG() {
}

s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
m.Lock()
clientManagers["postgres"] = s
m.Unlock()
}

func TestCreateGetDeleteClient(t *testing.T) {
Expand Down
18 changes: 9 additions & 9 deletions client/manager_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,22 @@ func TestHelperClientAutoGenerateKey(k string, m Storage) func(t *testing.T) {
return func(t *testing.T) {
t.Parallel()
c := &Client{
ID: "foo",
ClientID: "foo",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
TermsOfServiceURI: "foo",
}
assert.NoError(t, m.CreateClient(c))
//assert.NotEmpty(t, c.ID)
assert.NoError(t, m.DeleteClient(c.ID))
assert.NoError(t, m.DeleteClient(c.GetID()))
}
}

func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) {
return func(t *testing.T) {
t.Parallel()
m.CreateClient(&Client{
ID: "1234321",
ClientID: "1234321",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
})
Expand All @@ -59,7 +59,7 @@ func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) {

c, err = m.Authenticate("1234321", []byte("secret"))
require.NoError(t, err)
assert.Equal(t, "1234321", c.ID)
assert.Equal(t, "1234321", c.GetID())
}
}

Expand All @@ -70,7 +70,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
assert.NotNil(t, err)

c := &Client{
ID: "1234",
ClientID: "1234",
Name: "name",
Secret: "secret",
RedirectURIs: []string{"http://redirect", "http://redirect1"},
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
}

assert.NoError(t, m.CreateClient(&Client{
ID: "2-1234",
ClientID: "2-1234",
Name: "name",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
Expand All @@ -116,8 +116,8 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
ds, err := m.GetClients(100, 0)
assert.NoError(t, err)
assert.Len(t, ds, 2)
assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ID)
assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ClientID)
assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID)
assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID)

//test if SecretExpiresAt was set properly
assert.Equal(t, ds["1234"].SecretExpiresAt, 0)
Expand All @@ -132,7 +132,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
assert.Len(t, ds, 0)

err = m.UpdateClient(&Client{
ID: "2-1234",
ClientID: "2-1234",
Name: "name-new",
Secret: "secret-new",
RedirectURIs: []string{"http://redirect/new"},
Expand Down
11 changes: 2 additions & 9 deletions client/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func TestClientSDK(t *testing.T) {
t.Run("case=client is created and updated", func(t *testing.T) {
createClient := createTestClient("")
compareClient := createClient
compareClient.Id = compareClient.ClientId
createClient.ClientSecretExpiresAt = 10

// returned client is correct on Create
Expand Down Expand Up @@ -114,7 +113,6 @@ func TestClientSDK(t *testing.T) {
// create another client
updateClient := createTestClient("foo")
result, response, err = c.UpdateOAuth2Client(createClient.ClientId, updateClient)
updateClient.Id = updateClient.ClientId
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)
assert.EqualValues(t, updateClient, *result)
Expand All @@ -125,7 +123,6 @@ func TestClientSDK(t *testing.T) {
result, response, err = c.GetOAuth2Client(updateClient.ClientId)
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)
compareClient.ClientId = compareClient.Id
assert.EqualValues(t, compareClient, *result)

// client can not be found after being deleted
Expand Down Expand Up @@ -163,7 +160,7 @@ func TestClientSDK(t *testing.T) {
client: hydra.OAuth2Client{},
},
{
client: hydra.OAuth2Client{Id: "set-properly-1"},
client: hydra.OAuth2Client{ClientId: "set-properly-1"},
expectID: "set-properly-1",
},
{
Expand All @@ -176,13 +173,10 @@ func TestClientSDK(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, http.StatusCreated, response.StatusCode, "%s", response.Payload)

assert.NotEmpty(t, result.Id)
assert.NotEmpty(t, result.ClientId)
assert.EqualValues(t, result.Id, result.ClientId)

id := result.Id
id := result.ClientId
if tc.expectID != "" {
assert.EqualValues(t, tc.expectID, result.Id)
assert.EqualValues(t, tc.expectID, result.ClientId)
id = tc.expectID
}
Expand All @@ -191,7 +185,6 @@ func TestClientSDK(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)

assert.EqualValues(t, id, result.Id)
assert.EqualValues(t, id, result.ClientId)
})
}
Expand Down
6 changes: 1 addition & 5 deletions client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ func NewValidator(

func (v *Validator) Validate(c *Client) error {
id := uuid.New()
c.ID = stringsx.Coalesce(c.ID, c.ClientID, id)
c.ClientID = stringsx.Coalesce(c.ClientID, c.ID, id)
if c.ID != c.ClientID {
return errors.WithStack(fosite.ErrInvalidRequest.WithHint("Field id and client_id must match."))
}
c.ClientID = stringsx.Coalesce(c.ClientID, id)

if c.TokenEndpointAuthMethod == "" {
c.TokenEndpointAuthMethod = "client_secret_basic"
Expand Down
14 changes: 5 additions & 9 deletions client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,26 @@ func TestValidate(t *testing.T) {
in: new(Client),
check: func(t *testing.T, c *Client) {
assert.NotEmpty(t, c.ClientID)
assert.NotEmpty(t, c.ID)
assert.Equal(t, c.ID, c.ClientID)
assert.NotEmpty(t, c.GetID())
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ID: "foo"},
in: &Client{ClientID: "foo"},
check: func(t *testing.T, c *Client) {
assert.Equal(t, c.ID, c.ClientID)
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ClientID: "foo"},
check: func(t *testing.T, c *Client) {
assert.Equal(t, c.ID, c.ClientID)
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ClientID: "foo", UserinfoSignedResponseAlg: "foo"},
expectErr: true,
},
{
in: &Client{ClientID: "foo", ID: "bar"},
expectErr: true,
},
{
in: &Client{ClientID: "foo", TokenEndpointAuthMethod: "private_key_jwt"},
expectErr: true,
Expand Down
6 changes: 5 additions & 1 deletion cmd/cli/handler_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ import (

func checkResponse(response *hydra.APIResponse, err error, expectedStatusCode int) {
if response != nil {
pkg.Must(err, "Command failed because calling \"%s %s\" resulted in error \"%s\" occurred.\n%s\n", response.Request.Method, response.RequestURL, err, response.Payload)
var method string
if response.Response != nil && response.Response.Request != nil {
method = response.Request.Method
}
pkg.Must(err, "Command failed because calling \"%s %s\" resulted in error \"%s\" occurred.\n%s\n", method, response.RequestURL, err, response.Payload)
} else {
pkg.Must(err, "Command failed because error \"%s\" occurred and no response is available.\n", err)
}
Expand Down
Loading