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

refactor: introduce type resource.ID #704

Merged
merged 50 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
95051ee
move id into resource pkg
leg100 Nov 2, 2024
d203516
wip
leg100 Nov 2, 2024
a94ccd1
Merge branch 'master' into refactor-id
leg100 Nov 2, 2024
303f263
wip
leg100 Nov 2, 2024
792f2b7
wip
leg100 Nov 3, 2024
7dd6e98
wip
leg100 Nov 3, 2024
33cd71c
wip
leg100 Nov 3, 2024
3b46fdf
wip
leg100 Nov 3, 2024
03fdba2
wip
leg100 Nov 3, 2024
da9412a
wip
leg100 Nov 3, 2024
cd06f9d
wip
leg100 Nov 3, 2024
4790ac8
wip
leg100 Nov 4, 2024
9f8b2a9
Merge branch 'refactor-id' of github.com:leg100/otf into refactor-id
leg100 Nov 4, 2024
7795c48
wip
leg100 Nov 5, 2024
04c5979
wip
leg100 Nov 5, 2024
9364584
wip
leg100 Nov 5, 2024
9127826
wip
leg100 Nov 5, 2024
c2e877b
wip
leg100 Nov 5, 2024
41509da
wip
leg100 Nov 5, 2024
b5ec779
wip
leg100 Nov 5, 2024
12771f4
wip
leg100 Nov 5, 2024
560c3ef
wip
leg100 Nov 5, 2024
4f43358
wip
leg100 Nov 5, 2024
a744870
wip
leg100 Nov 5, 2024
81f6df8
wip
leg100 Nov 6, 2024
41e58ee
wip
leg100 Nov 6, 2024
7fab39c
wip
leg100 Nov 7, 2024
8e451cb
wip
leg100 Nov 7, 2024
cb5ded6
wip
leg100 Nov 7, 2024
f04b244
wip
leg100 Nov 7, 2024
69f2216
wip
leg100 Nov 7, 2024
e8b9916
wip
leg100 Nov 7, 2024
793fe69
wip
leg100 Nov 8, 2024
e151f66
wip
leg100 Nov 8, 2024
45f2b27
wip
leg100 Nov 8, 2024
ffbff9a
wip
leg100 Nov 8, 2024
f52c8cc
wip
leg100 Nov 8, 2024
352594f
wip
leg100 Nov 8, 2024
64ca438
wip
leg100 Nov 9, 2024
e9180b0
wip
leg100 Nov 9, 2024
8f18c56
wip
leg100 Nov 9, 2024
745a6d9
wip
leg100 Nov 9, 2024
d1bf204
wip
leg100 Nov 9, 2024
76180d9
wip
leg100 Nov 9, 2024
6123a58
wip
leg100 Nov 9, 2024
32e5921
wip
leg100 Nov 10, 2024
cdd3a9b
wip
leg100 Nov 10, 2024
a298c82
wip
leg100 Nov 10, 2024
5dd6685
wip
leg100 Nov 10, 2024
1173ff7
wip
leg100 Nov 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,11 @@ actions:
.PHONY: install-linter
install-linter:
go install honnef.co/go/tools/cmd/staticcheck@latest

.PHONY: debug
debug:
dlv debug --headless --api-version=2 --listen=127.0.0.1:4300 ./cmd/otfd/main.go

.PHONY: connect
connect:
dlv connect 127.0.0.1:4300 .
5 changes: 3 additions & 2 deletions internal/api/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/DataDog/jsonapi"
"github.com/leg100/otf/internal"
"github.com/leg100/otf/internal/resource"
"github.com/leg100/otf/internal/tfeapi/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -17,8 +18,8 @@ import (
func TestClient_UnmarshalResponse(t *testing.T) {
want := types.WorkspaceList{
Items: []*types.Workspace{
{ID: "ws-1", Outputs: []*types.WorkspaceOutput{}},
{ID: "ws-2", Outputs: []*types.WorkspaceOutput{}},
{ID: resource.NewID(resource.WorkspaceKind), Outputs: []*types.WorkspaceOutput{}},
{ID: resource.NewID(resource.WorkspaceKind), Outputs: []*types.WorkspaceOutput{}},
},
Pagination: &types.Pagination{},
}
Expand Down
25 changes: 18 additions & 7 deletions internal/authenticator/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,32 @@ import (
"context"
"net/http"

"github.com/leg100/otf/internal/tokens"
"github.com/leg100/otf/internal/resource"
"github.com/leg100/otf/internal/user"
"golang.org/x/oauth2"
)

type fakeTokenHandler struct {
username string
}
type fakeTokenHandler struct{}

func (f fakeTokenHandler) getUsername(ctx context.Context, token *oauth2.Token) (string, error) {
return f.username, nil
return "", nil
}

type fakeTokensService struct{}

func (fakeTokensService) StartSession(w http.ResponseWriter, r *http.Request, opts tokens.StartSessionOptions) error {
w.Header().Set("username", *opts.Username)
func (*fakeTokensService) StartSession(w http.ResponseWriter, r *http.Request, userID resource.ID) error {
w.Header().Set("user-id", userID.String())
return nil
}

type fakeUserService struct {
userID resource.ID
}

func (f *fakeUserService) GetUser(ctx context.Context, spec user.UserSpec) (*user.User, error) {
return &user.User{ID: f.userID}, nil
}

func (f *fakeUserService) Create(ctx context.Context, username string, opts ...user.NewUserOption) (*user.User, error) {
return nil, nil
}
38 changes: 27 additions & 11 deletions internal/authenticator/oauth_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import (

"github.com/gorilla/mux"
"github.com/leg100/otf/internal"
"github.com/leg100/otf/internal/authz"
"github.com/leg100/otf/internal/http/decode"
"github.com/leg100/otf/internal/http/html"
"github.com/leg100/otf/internal/http/html/paths"
"github.com/leg100/otf/internal/tokens"
"github.com/leg100/otf/internal/resource"
"github.com/leg100/otf/internal/user"
"golang.org/x/oauth2"
)

Expand All @@ -29,21 +31,21 @@ type (
}

sessionStarter interface {
StartSession(w http.ResponseWriter, r *http.Request, opts tokens.StartSessionOptions) error
StartSession(w http.ResponseWriter, r *http.Request, userID resource.ID) error
}

// OAuthClient performs the client role in an oauth handshake, requesting
// authorization from the user to access their account details on a particular
// cloud.
OAuthClient struct {
OAuthConfig
// extract username from token
tokenHandler
// for retrieving OTF system hostname to construct redirect URLs
*internal.HostnameService

OAuthConfig

sessions sessionStarter
users userService
}

// OAuthConfig is configuration for constructing an OAuth client
Expand All @@ -62,9 +64,9 @@ func newOAuthClient(
handler tokenHandler,
hostnameService *internal.HostnameService,
tokensService sessionStarter,
userService userService,
cfg OAuthConfig,
) (*OAuthClient, error) {

if cfg.ClientID == "" && cfg.ClientSecret != "" {
return nil, ErrOAuthCredentialsIncomplete
}
Expand All @@ -89,6 +91,7 @@ func newOAuthClient(
tokenHandler: handler,
HostnameService: hostnameService,
sessions: tokensService,
users: userService,
OAuthConfig: cfg,
}, nil
}
Expand Down Expand Up @@ -120,7 +123,9 @@ func (a *OAuthClient) requestHandler(w http.ResponseWriter, r *http.Request) {
// the code it receives for an access token, which it then uses to retrieve its
// corresponding username and start a new OTF user session.
func (a *OAuthClient) callbackHandler(w http.ResponseWriter, r *http.Request) {
getToken := func() (*oauth2.Token, error) {
// Get token; if there is an error, return user to login page along with
// flash error.
token, err := func() (*oauth2.Token, error) {
// Parse query string
var resp struct {
AuthCode string `schema:"code"`
Expand All @@ -147,10 +152,7 @@ func (a *OAuthClient) callbackHandler(w http.ResponseWriter, r *http.Request) {
// for testing purposes).
ctx := contextWithClient(r.Context(), a.SkipTLSVerification)
return a.config().Exchange(ctx, resp.AuthCode)
}
// Get token; if there is an error, return user to login page along with
// flash error.
token, err := getToken()
}()
if err != nil {
html.FlashError(w, err.Error())
http.Redirect(w, r, paths.Login(), http.StatusFound)
Expand All @@ -162,11 +164,25 @@ func (a *OAuthClient) callbackHandler(w http.ResponseWriter, r *http.Request) {
html.Error(w, err.Error(), http.StatusInternalServerError, false)
return
}
err = a.sessions.StartSession(w, r, tokens.StartSessionOptions{Username: &username})

// Retrieve user with extracted username, creating the user if necessary.
// Use privileged context to authorize access to user service endpoints.
ctx := authz.AddSubjectToContext(r.Context(), &authz.Superuser{Username: "oauth_client"})
user, err := a.users.GetUser(ctx, user.UserSpec{Username: &username})
if err == internal.ErrResourceNotFound {
user, err = a.users.Create(ctx, username)
}
if err != nil {
html.Error(w, err.Error(), http.StatusInternalServerError, false)
return
}

// Lookup user in db, to retrieve user id to embed in session token
// Get or create user.
if err := a.sessions.StartSession(w, r, user.ID); err != nil {
html.Error(w, err.Error(), http.StatusInternalServerError, false)
return
}
}

// config generates an oauth2 config for the client - note this is done at
Expand Down
13 changes: 8 additions & 5 deletions internal/authenticator/oauth_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"testing"

"github.com/leg100/otf/internal"
"github.com/leg100/otf/internal/resource"
"github.com/leg100/otf/internal/testutils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
)

func TestOAuthClient_requestHandler(t *testing.T) {
client := newTestOAuthServerClient(t, "")
client := newTestOAuthServerClient(t, testutils.ParseID(t, "user-bobby"))

r := httptest.NewRequest("GET", "/auth", nil)
w := httptest.NewRecorder()
Expand All @@ -32,18 +34,18 @@ func TestOAuthClient_requestHandler(t *testing.T) {
}

func TestOAuthClient_callbackHandler(t *testing.T) {
client := newTestOAuthServerClient(t, "bobby")
client := newTestOAuthServerClient(t, testutils.ParseID(t, "user-bobby"))
r := httptest.NewRequest("GET", "/auth?state=state", nil)
r.AddCookie(&http.Cookie{Name: oauthCookieName, Value: "state"})
w := httptest.NewRecorder()

client.callbackHandler(w, r)
assert.Equal(t, w.Header().Get("username"), "bobby")
assert.Equal(t, "user-bobby", w.Header().Get("user-id"))
}

// newTestOAuthServerClient creates an OAuth server for testing purposes and
// returns a client configured to access the server.
func newTestOAuthServerClient(t *testing.T, username string) *OAuthClient {
func newTestOAuthServerClient(t *testing.T, userID resource.ID) *OAuthClient {
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
out, err := json.Marshal(&oauth2.Token{AccessToken: "fake_token"})
require.NoError(t, err)
Expand All @@ -55,9 +57,10 @@ func newTestOAuthServerClient(t *testing.T, username string) *OAuthClient {
require.NoError(t, err)

client, err := newOAuthClient(
fakeTokenHandler{username},
fakeTokenHandler{},
internal.NewHostnameService("otf-server.com"),
&fakeTokensService{},
&fakeUserService{userID},
OAuthConfig{
Hostname: u.Host,
Endpoint: oauth2.Endpoint{
Expand Down
9 changes: 9 additions & 0 deletions internal/authenticator/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/leg100/otf/internal/http/html"
"github.com/leg100/otf/internal/logr"
"github.com/leg100/otf/internal/tokens"
"github.com/leg100/otf/internal/user"
)

type (
Expand All @@ -18,6 +19,7 @@ type (

*internal.HostnameService

UserService userService
TokensService *tokens.Service
OpaqueHandlerConfigs []OpaqueHandlerConfig
IDTokenHandlerConfig OIDCConfig
Expand All @@ -29,6 +31,11 @@ type (

clients []*OAuthClient
}

userService interface {
GetUser(ctx context.Context, spec user.UserSpec) (*user.User, error)
Create(ctx context.Context, username string, opts ...user.NewUserOption) (*user.User, error)
}
)

// NewAuthenticatorService constructs a service for logging users onto
Expand All @@ -47,6 +54,7 @@ func NewAuthenticatorService(ctx context.Context, opts Options) (*service, error
&opaqueHandler{cfg},
opts.HostnameService,
opts.TokensService,
opts.UserService,
cfg.OAuthConfig,
)
if err != nil {
Expand All @@ -69,6 +77,7 @@ func NewAuthenticatorService(ctx context.Context, opts Options) (*service, error
handler,
opts.HostnameService,
opts.TokensService,
opts.UserService,
OAuthConfig{
Endpoint: handler.provider.Endpoint(),
Scopes: opts.IDTokenHandlerConfig.Scopes,
Expand Down
3 changes: 2 additions & 1 deletion internal/authz/authorizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"context"

"github.com/leg100/otf/internal/rbac"
"github.com/leg100/otf/internal/resource"
)

// Authorizer is capable of granting or denying access to resources based on the
// subject contained within the context.
type Authorizer interface {
CanAccess(ctx context.Context, action rbac.Action, id string) (Subject, error)
CanAccess(ctx context.Context, action rbac.Action, id resource.ID) (Subject, error)
}
3 changes: 2 additions & 1 deletion internal/authz/authorizer_test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/leg100/otf/internal/rbac"
"github.com/leg100/otf/internal/resource"
)

type allowAllAuthorizer struct {
Expand All @@ -16,6 +17,6 @@ func NewAllowAllAuthorizer() *allowAllAuthorizer {
}
}

func (a *allowAllAuthorizer) CanAccess(context.Context, rbac.Action, string) (Subject, error) {
func (a *allowAllAuthorizer) CanAccess(context.Context, rbac.Action, resource.ID) (Subject, error) {
return a.User, nil
}
13 changes: 7 additions & 6 deletions internal/authz/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"

"github.com/leg100/otf/internal/rbac"
"github.com/leg100/otf/internal/resource"
)

// unexported key types prevents collisions
Expand All @@ -22,7 +23,7 @@ const (
// Subject is an entity that carries out actions on resources.
type Subject interface {
CanAccessSite(action rbac.Action) bool
CanAccessTeam(action rbac.Action, id string) bool
CanAccessTeam(action rbac.Action, id resource.ID) bool
CanAccessOrganization(action rbac.Action, name string) bool
CanAccessWorkspace(action rbac.Action, policy WorkspacePolicy) bool

Expand All @@ -37,7 +38,7 @@ type Subject interface {
// WorkspacePolicy binds workspace permissions to a workspace
type WorkspacePolicy struct {
Organization string
WorkspaceID string
WorkspaceID resource.ID
Permissions []WorkspacePermission

// Whether workspace permits its state to be consumed by all workspaces in
Expand All @@ -47,7 +48,7 @@ type WorkspacePolicy struct {

// WorkspacePermission binds a role to a team.
type WorkspacePermission struct {
TeamID string
TeamID resource.ID
Role rbac.Role
}

Expand Down Expand Up @@ -86,12 +87,12 @@ type Superuser struct {
}

func (*Superuser) CanAccessSite(action rbac.Action) bool { return true }
func (*Superuser) CanAccessTeam(rbac.Action, string) bool { return true }
func (*Superuser) CanAccessTeam(rbac.Action, resource.ID) bool { return true }
func (*Superuser) CanAccessOrganization(rbac.Action, string) bool { return true }
func (*Superuser) CanAccessWorkspace(rbac.Action, WorkspacePolicy) bool { return true }
func (s *Superuser) Organizations() []string { return nil }
func (s *Superuser) String() string { return s.Username }
func (s *Superuser) ID() string { return s.Username }
func (s *Superuser) GetID() resource.ID { return resource.NewID(resource.UserKind) }
func (s *Superuser) IsSiteAdmin() bool { return true }
func (s *Superuser) IsOwner(string) bool { return true }

Expand All @@ -101,7 +102,7 @@ type Nobody struct {
}

func (*Nobody) CanAccessSite(action rbac.Action) bool { return false }
func (*Nobody) CanAccessTeam(rbac.Action, string) bool { return false }
func (*Nobody) CanAccessTeam(rbac.Action, resource.ID) bool { return false }
func (*Nobody) CanAccessOrganization(rbac.Action, string) bool { return false }
func (*Nobody) CanAccessWorkspace(rbac.Action, WorkspacePolicy) bool { return false }
func (s *Nobody) Organizations() []string { return nil }
Expand Down
3 changes: 2 additions & 1 deletion internal/authz/site_authorizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import (
"github.com/go-logr/logr"
"github.com/leg100/otf/internal"
"github.com/leg100/otf/internal/rbac"
"github.com/leg100/otf/internal/resource"
)

// SiteAuthorizer authorizes access to site-wide actions
type SiteAuthorizer struct {
logr.Logger
}

func (a *SiteAuthorizer) CanAccess(ctx context.Context, action rbac.Action, _ string) (Subject, error) {
func (a *SiteAuthorizer) CanAccess(ctx context.Context, action rbac.Action, _ resource.ID) (Subject, error) {
subj, err := SubjectFromContext(ctx)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion internal/configversion/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (a *api) addHandlers(r *mux.Router) {
}

func (a *api) download(w http.ResponseWriter, r *http.Request) {
id, err := decode.Param("id", r)
id, err := decode.ID("id", r)
if err != nil {
tfeapi.Error(w, err)
return
Expand Down
Loading
Loading