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

Support github refresh tokens #3811

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
13 changes: 7 additions & 6 deletions server/api/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,14 @@

// create the user account
user = &model.User{
Login: userFromForge.Login,
ForgeID: forgeID,

Check warning on line 108 in server/api/login.go

View check run for this annotation

Codecov / codecov/patch

server/api/login.go#L108

Added line #L108 was not covered by tests
ForgeRemoteID: userFromForge.ForgeRemoteID,
Token: userFromForge.Token,
Secret: userFromForge.Secret,
Login: userFromForge.Login,
AccessToken: userFromForge.AccessToken,
RefreshToken: userFromForge.RefreshToken,
Expiry: userFromForge.Expiry,

Check warning on line 113 in server/api/login.go

View check run for this annotation

Codecov / codecov/patch

server/api/login.go#L110-L113

Added lines #L110 - L113 were not covered by tests
Email: userFromForge.Email,
Avatar: userFromForge.Avatar,
ForgeID: forgeID,
Hash: base32.StdEncoding.EncodeToString(
securecookie.GenerateRandomKey(32),
),
Expand Down Expand Up @@ -165,8 +166,8 @@
}

// update the user meta data and authorization data.
user.Token = userFromForge.Token
user.Secret = userFromForge.Secret
user.AccessToken = userFromForge.AccessToken
user.RefreshToken = userFromForge.RefreshToken

Check warning on line 170 in server/api/login.go

View check run for this annotation

Codecov / codecov/patch

server/api/login.go#L169-L170

Added lines #L169 - L170 were not covered by tests
user.Email = userFromForge.Email
user.Avatar = userFromForge.Avatar
user.ForgeRemoteID = userFromForge.ForgeRemoteID
Expand Down
8 changes: 4 additions & 4 deletions server/forge/addon/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@

func (m *modelUser) asModel() *model.User {
m.User.ForgeRemoteID = m.ForgeRemoteID
m.User.Token = m.Token
m.User.Secret = m.Secret
m.User.AccessToken = m.Token
m.User.RefreshToken = m.Secret

Check warning on line 115 in server/forge/addon/args.go

View check run for this annotation

Codecov / codecov/patch

server/forge/addon/args.go#L114-L115

Added lines #L114 - L115 were not covered by tests
m.User.Expiry = m.Expiry
m.User.Hash = m.Hash
return m.User
Expand All @@ -122,8 +122,8 @@
return &modelUser{
User: u,
ForgeRemoteID: u.ForgeRemoteID,
Token: u.Token,
Secret: u.Secret,
Token: u.AccessToken,
Secret: u.RefreshToken,

Check warning on line 126 in server/forge/addon/args.go

View check run for this annotation

Codecov / codecov/patch

server/forge/addon/args.go#L125-L126

Added lines #L125 - L126 were not covered by tests
Expiry: u.Expiry,
Hash: u.Hash,
}
Expand Down
10 changes: 5 additions & 5 deletions server/forge/bitbucket/bitbucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,15 @@ func (c *config) Auth(ctx context.Context, token, secret string) (string, error)
func (c *config) Refresh(ctx context.Context, user *model.User) (bool, error) {
config := c.newOAuth2Config()
source := config.TokenSource(
ctx, &oauth2.Token{RefreshToken: user.Secret})
ctx, &oauth2.Token{RefreshToken: user.RefreshToken})

token, err := source.Token()
if err != nil || len(token.AccessToken) == 0 {
return false, err
}

user.Token = token.AccessToken
user.Secret = token.RefreshToken
user.AccessToken = token.AccessToken
user.RefreshToken = token.RefreshToken
user.Expiry = token.Expiry.UTC().Unix()
return true, nil
}
Expand Down Expand Up @@ -348,7 +348,7 @@ func (c *config) Netrc(u *model.User, _ *model.Repo) (*model.Netrc, error) {
return &model.Netrc{
Machine: "bitbucket.org",
Login: "x-token-auth",
Password: u.Token,
Password: u.AccessToken,
}, nil
}

Expand Down Expand Up @@ -428,7 +428,7 @@ func (c *config) newClient(ctx context.Context, u *model.User) *internal.Client
if u == nil {
return c.newClientToken(ctx, "", "")
}
return c.newClientToken(ctx, u.Token, u.Secret)
return c.newClientToken(ctx, u.AccessToken, u.RefreshToken)
}

// helper function to return the bitbucket oauth2 client.
Expand Down
42 changes: 21 additions & 21 deletions server/forge/bitbucket/bitbucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func Test_bitbucket(t *testing.T) {
netrc, _ := forge.Netrc(fakeUser, fakeRepo)
g.Assert(netrc.Machine).Equal("bitbucket.org")
g.Assert(netrc.Login).Equal("x-token-auth")
g.Assert(netrc.Password).Equal(fakeUser.Token)
g.Assert(netrc.Password).Equal(fakeUser.AccessToken)
})

g.Describe("Given an authorization request", func() {
Expand All @@ -75,8 +75,8 @@ func Test_bitbucket(t *testing.T) {
})
g.Assert(err).IsNil()
g.Assert(u.Login).Equal(fakeUser.Login)
g.Assert(u.Token).Equal("2YotnFZFEjr1zCsicMWpAA")
g.Assert(u.Secret).Equal("tGzv3JOkF0XG5Qx2TlKWIA")
g.Assert(u.AccessToken).Equal("2YotnFZFEjr1zCsicMWpAA")
g.Assert(u.RefreshToken).Equal("tGzv3JOkF0XG5Qx2TlKWIA")
})
g.It("Should handle failure to exchange code", func() {
_, _, err := c.Login(ctx, &types.OAuthRequest{
Expand All @@ -94,12 +94,12 @@ func Test_bitbucket(t *testing.T) {

g.Describe("Given an access token", func() {
g.It("Should return the authenticated user", func() {
login, err := c.Auth(ctx, fakeUser.Token, fakeUser.Secret)
login, err := c.Auth(ctx, fakeUser.AccessToken, fakeUser.RefreshToken)
g.Assert(err).IsNil()
g.Assert(login).Equal(fakeUser.Login)
})
g.It("Should handle a failure to resolve user", func() {
_, err := c.Auth(ctx, fakeUserNotFound.Token, fakeUserNotFound.Secret)
_, err := c.Auth(ctx, fakeUserNotFound.AccessToken, fakeUserNotFound.RefreshToken)
g.Assert(err).IsNotNil()
})
})
Expand All @@ -109,8 +109,8 @@ func Test_bitbucket(t *testing.T) {
ok, err := c.Refresh(ctx, fakeUserRefresh)
g.Assert(err).IsNil()
g.Assert(ok).IsTrue()
g.Assert(fakeUserRefresh.Token).Equal("2YotnFZFEjr1zCsicMWpAA")
g.Assert(fakeUserRefresh.Secret).Equal("tGzv3JOkF0XG5Qx2TlKWIA")
g.Assert(fakeUserRefresh.AccessToken).Equal("2YotnFZFEjr1zCsicMWpAA")
g.Assert(fakeUserRefresh.RefreshToken).Equal("tGzv3JOkF0XG5Qx2TlKWIA")
})
g.It("Should handle an empty access token", func() {
ok, err := c.Refresh(ctx, fakeUserRefreshEmpty)
Expand Down Expand Up @@ -293,38 +293,38 @@ func Test_bitbucket(t *testing.T) {

var (
fakeUser = &model.User{
Login: "superman",
Token: "cfcd2084",
Login: "superman",
AccessToken: "cfcd2084",
}

fakeUserRefresh = &model.User{
Login: "superman",
Secret: "cfcd2084",
Login: "superman",
RefreshToken: "cfcd2084",
}

fakeUserRefreshFail = &model.User{
Login: "superman",
Secret: "refresh_token_not_found",
Login: "superman",
RefreshToken: "refresh_token_not_found",
}

fakeUserRefreshEmpty = &model.User{
Login: "superman",
Secret: "refresh_token_is_empty",
Login: "superman",
RefreshToken: "refresh_token_is_empty",
}

fakeUserNotFound = &model.User{
Login: "superman",
Token: "user_not_found",
Login: "superman",
AccessToken: "user_not_found",
}

fakeUserNoTeams = &model.User{
Login: "superman",
Token: "teams_not_found",
Login: "superman",
AccessToken: "teams_not_found",
}

fakeUserNoRepos = &model.User{
Login: "superman",
Token: "repos_not_found",
Login: "superman",
AccessToken: "repos_not_found",
}

fakeRepo = &model.Repo{
Expand Down
4 changes: 2 additions & 2 deletions server/forge/bitbucket/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ func sshCloneLink(repo *internal.Repo) string {
func convertUser(from *internal.Account, token *oauth2.Token) *model.User {
return &model.User{
Login: from.Login,
Token: token.AccessToken,
Secret: token.RefreshToken,
AccessToken: token.AccessToken,
RefreshToken: token.RefreshToken,
Expiry: token.Expiry.UTC().Unix(),
Avatar: from.Links.Avatar.Href,
ForgeRemoteID: model.ForgeRemoteID(fmt.Sprint(from.UUID)),
Expand Down
4 changes: 2 additions & 2 deletions server/forge/bitbucket/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ func Test_helper(t *testing.T) {
result := convertUser(user, token)
g.Assert(result.Avatar).Equal(user.Links.Avatar.Href)
g.Assert(result.Login).Equal(user.Login)
g.Assert(result.Token).Equal(token.AccessToken)
g.Assert(result.Secret).Equal(token.RefreshToken)
g.Assert(result.AccessToken).Equal(token.AccessToken)
g.Assert(result.RefreshToken).Equal(token.RefreshToken)
g.Assert(result.Expiry).Equal(token.Expiry.UTC().Unix())
})

Expand Down
6 changes: 3 additions & 3 deletions server/forge/bitbucketdatacenter/bitbucketdatacenter.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
return nil, "", err
}

bc, err := c.newClient(ctx, &model.User{Token: token.AccessToken})
bc, err := c.newClient(ctx, &model.User{AccessToken: token.AccessToken})

Check warning on line 119 in server/forge/bitbucketdatacenter/bitbucketdatacenter.go

View check run for this annotation

Codecov / codecov/patch

server/forge/bitbucketdatacenter/bitbucketdatacenter.go#L119

Added line #L119 was not covered by tests
if err != nil {
return nil, "", fmt.Errorf("unable to create bitbucket client: %w", err)
}
Expand All @@ -143,7 +143,7 @@
func (c *client) Refresh(ctx context.Context, u *model.User) (bool, error) {
config := c.newOAuth2Config()
t := &oauth2.Token{
RefreshToken: u.Secret,
RefreshToken: u.RefreshToken,

Check warning on line 146 in server/forge/bitbucketdatacenter/bitbucketdatacenter.go

View check run for this annotation

Codecov / codecov/patch

server/forge/bitbucketdatacenter/bitbucketdatacenter.go#L146

Added line #L146 was not covered by tests
}
ts := config.TokenSource(ctx, t)

Expand Down Expand Up @@ -623,7 +623,7 @@
func (c *client) newClient(ctx context.Context, u *model.User) (*bb.Client, error) {
config := c.newOAuth2Config()
t := &oauth2.Token{
AccessToken: u.Token,
AccessToken: u.AccessToken,
}
client := config.Client(ctx, t)
return bb.NewClient(c.urlAPI, client)
Expand Down
4 changes: 2 additions & 2 deletions server/forge/bitbucketdatacenter/bitbucketdatacenter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ func TestBitbucketDC(t *testing.T) {
}

var fakeUser = &model.User{
Token: "fake",
Expiry: time.Now().Add(1 * time.Hour).Unix(),
AccessToken: "fake",
Expiry: time.Now().Add(1 * time.Hour).Unix(),
}
4 changes: 2 additions & 2 deletions server/forge/bitbucketdatacenter/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
}

func updateUserCredentials(u *model.User, t *oauth2.Token) {
u.Token = t.AccessToken
u.Secret = t.RefreshToken
u.AccessToken = t.AccessToken
u.RefreshToken = t.RefreshToken

Check warning on line 172 in server/forge/bitbucketdatacenter/convert.go

View check run for this annotation

Codecov / codecov/patch

server/forge/bitbucketdatacenter/convert.go#L171-L172

Added lines #L171 - L172 were not covered by tests
u.Expiry = t.Expiry.UTC().Unix()
}
4 changes: 2 additions & 2 deletions server/forge/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@

func UserToken(ctx context.Context, r *model.Repo, u *model.User) string {
if u != nil {
return u.Token
return u.AccessToken

Check warning on line 49 in server/forge/common/utils.go

View check run for this annotation

Codecov / codecov/patch

server/forge/common/utils.go#L49

Added line #L49 was not covered by tests
}

_store, ok := store.TryFromContext(ctx)
Expand All @@ -62,5 +62,5 @@
if err != nil {
return ""
}
return user.Token
return user.AccessToken

Check warning on line 65 in server/forge/common/utils.go

View check run for this annotation

Codecov / codecov/patch

server/forge/common/utils.go#L65

Added line #L65 was not covered by tests
}
Loading