Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@JEFFTheDev
JEFFTheDev committed Dec 28, 2023
1 parent ffcc53e commit 061120a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 32 deletions.
20 changes: 13 additions & 7 deletions services/tenants/apikeys/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,17 @@ func (s *service) AuthenticateApiKey(base64IdAndKeyCombination string) (ApiKeyAu
}
isValid := hashed.compare(apiKey)
if isValid {
return ApiKeyAuthenticationDTO{
TenantID: hashed.TenantID,
}, nil
if hashed.ExpirationDate != nil {
exp := hashed.ExpirationDate.Unix()
return ApiKeyAuthenticationDTO{
TenantID: hashed.TenantID,
Expiration: &exp,
}, nil
} else {
return ApiKeyAuthenticationDTO{
TenantID: hashed.TenantID,
}, nil
}
}
return ApiKeyAuthenticationDTO{}, ErrKeyNotFound
}
Expand All @@ -94,10 +102,8 @@ type Filter struct {
}

type ApiKeyAuthenticationDTO struct {
TenantID int64 `json:"sub"` // Sub is how Ory Oathkeeper identifies the important information in the response
Expiration time.Duration
// TODO: check, how to make ory oathkeeper only use the first value and not 'refresh' this expiration each time a request is authenticated?
// TODO: is there a diference in flow: just check existing token or get new token?
TenantID int64 `json:"sub"` // Sub is how Ory Oathkeeper identifies the important information in the response
Expiration *int64 `json:"expiration_date"`
}

type ApiKeyDTO struct {
Expand Down
77 changes: 52 additions & 25 deletions services/tenants/transports/apikeys_http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestNewApiKeyInvalidJsonBody(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`blablabla`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`blablabla`))

// Act
rr := httptest.NewRecorder()
Expand All @@ -35,7 +35,7 @@ func TestNewApiKeyNoName(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`{"name": "", "organisation_id": 905}`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`{"name": "", "organisation_id": 905}`))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -52,7 +52,7 @@ func TestNewApiKeyNoOrganisationID(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`{"name": "wasdasdas", "organisation_id": 0}`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`{"name": "wasdasdas", "organisation_id": 0}`))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -69,7 +69,7 @@ func TestNewApiKeyExpirationDateNotInTheFuture(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys",
req, _ := http.NewRequest("POST", "/tenants/api-keys",
strings.NewReader(fmt.Sprintf(`{"name": "wasdasdas", "organisation_id": 12, "expiration_date": "%s"}`, time.Now().Add(-time.Hour*24).Format(time.RFC3339))))
req.Header.Add("content-type", "application/json")

Expand All @@ -93,7 +93,7 @@ func TestNewApiKeyTenantIsNotFound(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -116,7 +116,7 @@ func TestNewApiKeyErrorOccurs(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -141,7 +141,7 @@ func TestNewApiKeyIsCreatedWithExpirationDate(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(fmt.Sprintf(`{"name": "whatever", "organisation_id": 905, "expiration_date": "%s"}`, exp.Format("2006-01-02T15:04:05.999999999Z"))))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(fmt.Sprintf(`{"name": "whatever", "organisation_id": 905, "expiration_date": "%s"}`, exp.Format("2006-01-02T15:04:05.999999999Z"))))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -164,7 +164,7 @@ func TestNewApiKeyIsCreatedWithoutExpirationDate(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("POST", "/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req, _ := http.NewRequest("POST", "/tenants/api-keys", strings.NewReader(`{"name": "whatever", "organisation_id": 905}`))
req.Header.Add("content-type", "application/json")

// Act
Expand All @@ -181,7 +181,7 @@ func TestRevokeApiKeyInvalidApiKeyId(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("DELETE", "/api-keys/blablalb", nil)
req, _ := http.NewRequest("DELETE", "/tenants/api-keys/blablalb", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -202,7 +202,7 @@ func TestRevokeApiKeyRevokesApiKey(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("DELETE", "/api-keys/123", nil)
req, _ := http.NewRequest("DELETE", "/tenants/api-keys/123", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -223,7 +223,7 @@ func TestRevokeApiKeyRevokeFails(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("DELETE", "/api-keys/12343", nil)
req, _ := http.NewRequest("DELETE", "/tenants/api-keys/12343", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -244,7 +244,7 @@ func TestRevokeApiKeyKeyDoesNotExist(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("DELETE", "/api-keys/12343", nil)
req, _ := http.NewRequest("DELETE", "/tenants/api-keys/12343", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -260,7 +260,7 @@ func TestAuthenticateNoAuthorizationHeaderInRequest(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -275,7 +275,7 @@ func TestAuthenticateAuthorizationHeaderIncorrectFormat(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"wrong format!!"}
Expand All @@ -296,7 +296,7 @@ func TestAuthenticateErrorOccursWhileValidatingApiKey(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"Bearer MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ=="}
Expand All @@ -318,7 +318,7 @@ func TestAuthenticateApiKeyIsNotFound(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"Bearer MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ=="}
Expand All @@ -340,7 +340,7 @@ func TestAuthenticateApiKeyInvalidEncodingErrorOccurs(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"Bearer blablabla"}
Expand All @@ -353,18 +353,19 @@ func TestAuthenticateApiKeyInvalidEncodingErrorOccurs(t *testing.T) {
assert.Len(t, svc.AuthenticateApiKeyCalls(), 1)
}

func TestAuthenticateApiKeyIsValid(t *testing.T) {
func TestAuthenticateApiKeyIsValidNoExpirationDate(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{
AuthenticateApiKeyFunc: func(base64IdAndKeyCombination string) (apikeys.ApiKeyAuthenticationDTO, error) {
assert.Equal(t, "MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ==", base64IdAndKeyCombination)
return apikeys.ApiKeyAuthenticationDTO{
TenantID: 431,
TenantID: 431,
Expiration: nil,
}, nil
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/authenticate", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"Bearer MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ=="}
Expand All @@ -373,7 +374,33 @@ func TestAuthenticateApiKeyIsValid(t *testing.T) {

// Assert
assert.Equal(t, http.StatusOK, rr.Code)
assert.Equal(t, `{"sub":431}`+"\n", rr.Body.String())
assert.Equal(t, `{"sub":431,"expiration_date":null}`+"\n", rr.Body.String())
assert.Len(t, svc.AuthenticateApiKeyCalls(), 1)
}

func TestAuthenticateApiKeyIsValidWithExpirationDate(t *testing.T) {
// Arrange
exp := time.Now().Add(time.Minute).Unix()
svc := apiKeyServiceMock{
AuthenticateApiKeyFunc: func(base64IdAndKeyCombination string) (apikeys.ApiKeyAuthenticationDTO, error) {
assert.Equal(t, "MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ==", base64IdAndKeyCombination)
return apikeys.ApiKeyAuthenticationDTO{
TenantID: 431,
Expiration: &exp,
}, nil
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/tenants/api-keys/authenticate", nil)

// Act
req.Header["Authorization"] = []string{"Bearer MjMxNDMyNDM6bXl2YWxpZGFwaWtleQ=="}
rr := httptest.NewRecorder()
transport.ServeHTTP(rr, req)

// Assert
assert.Equal(t, http.StatusOK, rr.Code)
assert.Equal(t, fmt.Sprintf(`{"sub":431,"expiration_date":%d}`+"\n", exp), rr.Body.String())
assert.Len(t, svc.AuthenticateApiKeyCalls(), 1)
}

Expand All @@ -395,7 +422,7 @@ func TestListApiKeysReturnsPaginatedList(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/list", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/list", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -404,15 +431,15 @@ func TestListApiKeysReturnsPaginatedList(t *testing.T) {
// Assert
assert.Equal(t, http.StatusOK, rr.Code)
assert.Equal(t,
`{"links":{"previous":"","next":"/api-keys/list?cursor=encoded_cursor"},"page_size":2,"total_count":0,"data":[{"name":"api-key-1","tenant_id":0,"expiration_date":null},{"name":"api-key-2","tenant_id":0,"expiration_date":null}]}`+"\n", rr.Body.String())
`{"links":{"previous":"","next":"/tenants/api-keys/list?cursor=encoded_cursor"},"page_size":2,"total_count":0,"data":[{"name":"api-key-1","tenant_id":0,"expiration_date":null},{"name":"api-key-2","tenant_id":0,"expiration_date":null}]}`+"\n", rr.Body.String())
assert.Len(t, svc.ListAPIKeysCalls(), 1)
}

func TestListApiKeysInvalidParams(t *testing.T) {
// Arrange
svc := apiKeyServiceMock{}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/list?tenant_id=blablalq", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/list?tenant_id=blablalq", nil)

// Act
rr := httptest.NewRecorder()
Expand All @@ -432,7 +459,7 @@ func TestListApiKeysErrorsOccursWhileRetrievingData(t *testing.T) {
},
}
transport := testTransport(&svc)
req, _ := http.NewRequest("GET", "/api-keys/list", nil)
req, _ := http.NewRequest("GET", "/tenants/api-keys/list", nil)

// Act
rr := httptest.NewRecorder()
Expand Down

0 comments on commit 061120a

Please sign in to comment.