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.

Sign up for GitHub

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

chore: Migrate to use golang-jwt/jwt v4.2.0 #8136

Merged
merged 3 commits into from
Jan 13, 2022
Merged

chore: Migrate to use golang-jwt/jwt v4.2.0 #8136

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8c0ac1
chore: Migrate to use golang-jwt/jwt v4.2.0
terrytangyuan Jan 10, 2022
2621919
chore: Use the new methods
terrytangyuan Jan 11, 2022
9f21e8e
chore: fix lint
terrytangyuan Jan 11, 2022
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
2 changes: 1 addition & 1 deletion cmd/argocd/commands/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"time"

"github.com/coreos/go-oidc"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"
"github.com/skratchdot/open-golang/open"
"github.com/spf13/cobra"
Expand Down
2 changes: 1 addition & 1 deletion cmd/argocd/commands/login_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package commands
import (
"testing"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
)

Expand Down
2 changes: 1 addition & 1 deletion cmd/argocd/commands/project_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"time"

timeutil "github.com/argoproj/pkg/time"
jwtgo "github.com/dgrijalva/jwt-go/v4"
jwtgo "github.com/golang-jwt/jwt/v4"
"github.com/spf13/cobra"

argocdclient "github.com/argoproj/argo-cd/v2/pkg/apiclient"
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/casbin/casbin/v2 v2.39.1
github.com/chai2010/gettext-go v0.0.0-20170215093142-bf70f2a70fb1 // indirect
github.com/coreos/go-oidc v2.1.0+incompatible
github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1
github.com/dustin/go-humanize v1.0.0
github.com/evanphx/json-patch v4.11.0+incompatible
github.com/fsnotify/fsnotify v1.4.9
Expand All @@ -32,6 +31,7 @@ require (
github.com/gobwas/glob v0.2.3
github.com/gogits/go-gogs-client v0.0.0-20190616193657-5a05380e4bc2
github.com/gogo/protobuf v1.3.2
github.com/golang-jwt/jwt/v4 v4.2.0
github.com/golang/protobuf v1.5.2
github.com/gomodule/redigo v2.0.0+incompatible // indirect
github.com/google/go-cmp v0.5.6
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV
github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
github.com/golang-jwt/jwt/v4 v4.2.0 h1:besgBTC8w8HjP6NzQdxwKH9Z5oQMZ24ThTrHp3cZ8eU=
github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiclient/apiclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"time"

"github.com/coreos/go-oidc"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/golang/protobuf/ptypes/empty"
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
Expand Down
2 changes: 1 addition & 1 deletion server/account/account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"testing"
"time"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down
2 changes: 1 addition & 1 deletion server/application/application_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/argoproj/gitops-engine/pkg/utils/kube/kubetest"
"github.com/argoproj/pkg/sync"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/ghodss/yaml"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down
2 changes: 1 addition & 1 deletion server/logout/logout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"strings"
"time"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"

"github.com/argoproj/argo-cd/v2/common"
Expand Down
2 changes: 1 addition & 1 deletion server/logout/logout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/argoproj/argo-cd/v2/util/session"
"github.com/argoproj/argo-cd/v2/util/settings"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down
2 changes: 1 addition & 1 deletion server/project/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/argoproj/argo-cd/v2/util/db"

"github.com/argoproj/pkg/sync"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/google/uuid"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
Expand Down
2 changes: 1 addition & 1 deletion server/project/project_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/argoproj/argo-cd/v2/util/db"

"github.com/argoproj/pkg/sync"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"
Expand Down
2 changes: 1 addition & 1 deletion server/rbacpolicy/rbacpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package rbacpolicy
import (
"strings"

jwt "github.com/dgrijalva/jwt-go/v4"
jwt "github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"

"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
Expand Down
2 changes: 1 addition & 1 deletion server/rbacpolicy/rbacpolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"testing"

jwt "github.com/dgrijalva/jwt-go/v4"
jwt "github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
Expand Down
2 changes: 1 addition & 1 deletion server/repository/repository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (

"github.com/stretchr/testify/assert"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand Down
2 changes: 1 addition & 1 deletion server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
golang_proto "github.com/golang/protobuf/proto"

"github.com/argoproj/pkg/sync"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/go-redis/redis/v8"
"github.com/gorilla/handlers"
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
Expand Down
2 changes: 1 addition & 1 deletion server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"testing"
"time"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/metadata"
Expand Down
2 changes: 1 addition & 1 deletion util/clusterauth/clusterauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"strings"
"time"

jwt "github.com/dgrijalva/jwt-go/v4"
jwt "github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
Expand Down
2 changes: 1 addition & 1 deletion util/jwt/jwt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strings"
"time"

jwtgo "github.com/dgrijalva/jwt-go/v4"
jwtgo "github.com/golang-jwt/jwt/v4"
)

// MapClaims converts a jwt.Claims to a MapClaims
Expand Down
2 changes: 1 addition & 1 deletion util/jwt/jwt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"testing"
"time"

jwt "github.com/dgrijalva/jwt-go/v4"
jwt "github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
)

Expand Down
10 changes: 4 additions & 6 deletions util/localconfig/localconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"path"
"strings"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"

configUtil "github.com/argoproj/argo-cd/v2/util/config"
)
Expand Down Expand Up @@ -65,11 +65,9 @@ type User struct {
}

// Claims returns the standard claims from the JWT claims
func (u *User) Claims() (*jwt.StandardClaims, error) {
parser := &jwt.Parser{
ValidationHelper: jwt.NewValidationHelper(jwt.WithoutClaimsValidation(), jwt.WithoutAudienceValidation()),
}
claims := jwt.StandardClaims{}
func (u *User) Claims() (*jwt.RegisteredClaims, error) {
parser := jwt.NewParser(jwt.WithoutClaimsValidation())
claims := jwt.RegisteredClaims{}
_, _, err := parser.ParseUnverified(u.AuthToken, &claims)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion util/oidc/oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"time"

gooidc "github.com/coreos/go-oidc"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"
"golang.org/x/oauth2"

Expand Down
2 changes: 1 addition & 1 deletion util/rbac/rbac.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/casbin/casbin/v2"
"github.com/casbin/casbin/v2/model"
"github.com/casbin/casbin/v2/util"
jwt "github.com/dgrijalva/jwt-go/v4"
jwt "github.com/golang-jwt/jwt/v4"
gocache "github.com/patrickmn/go-cache"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
Expand Down
2 changes: 1 addition & 1 deletion util/rbac/rbac_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"testing"
"time"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
apiv1 "k8s.io/api/core/v1"
Expand Down
44 changes: 8 additions & 36 deletions util/session/sessionmanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package session

import (
"context"
"encoding/json"
"errors"
"fmt"
"math"
Expand All @@ -14,7 +13,7 @@ import (
"time"

oidc "github.com/coreos/go-oidc"
"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/google/uuid"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -157,16 +156,16 @@ func (mgr *SessionManager) Create(subject string, secondsBeforeExpiry int64, id
// Create a new token object, specifying signing method and the claims
// you would like it to contain.
now := time.Now().UTC()
claims := jwt.StandardClaims{
IssuedAt: jwt.At(now),
claims := jwt.RegisteredClaims{
IssuedAt: jwt.NewNumericDate(now),
Issuer: SessionManagerClaimsIssuer,
NotBefore: jwt.At(now),
NotBefore: jwt.NewNumericDate(now),
Subject: subject,
ID: id,
}
if secondsBeforeExpiry > 0 {
expires := now.Add(time.Duration(secondsBeforeExpiry) * time.Second)
claims.ExpiresAt = jwt.At(expires)
claims.ExpiresAt = jwt.NewNumericDate(expires)
}

return mgr.signClaims(claims)
Expand All @@ -182,38 +181,13 @@ type standardClaims struct {
Subject string `json:"sub,omitempty"`
}

func unixTimeOrZero(t *jwt.Time) int64 {
if t == nil {
return 0
}
return t.Unix()
}

func (mgr *SessionManager) signClaims(claims jwt.Claims) (string, error) {
// log.Infof("Issuing claims: %v", claims)
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
settings, err := mgr.settingsMgr.GetSettings()
if err != nil {
return "", err
}
// workaround for https://github.com/argoproj/argo-cd/issues/5217
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexmt Since you originally added these in #5228. Could you help review this removal? Looks like since we are using the new method jwt.NewNumericDate, the precision will be in seconds.

// TimePrecision sets the precision of times and dates within this library.
// This has an influence on the precision of times when comparing expiry or
// other related time fields. Furthermore, it is also the precision of times
// when serializing.
//
// For backwards compatibility the default precision is set to seconds, so that
// no fractional timestamps are generated.
var TimePrecision = time.Second

// NumericDate represents a JSON numeric date value, as referenced at
// https://datatracker.ietf.org/doc/html/rfc7519#section-2.
type NumericDate struct {
	time.Time
}

// NewNumericDate constructs a new *NumericDate from a standard library time.Time struct.
// It will truncate the timestamp according to the precision specified in TimePrecision.
func NewNumericDate(t time.Time) *NumericDate {
	return &NumericDate{t.Truncate(TimePrecision)}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the workaround that Alex has implemented was due to the fact that our JWTs contain integer timestamps, while the library worked with floats. Now they seem to support multiple precisions and have reverted to integers for the precision we require (seconds), which is nice :) To me, the removal of this workaround looks good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jannfis Thanks for confirming! cc @alexmt to double check as well.

// According to https://tools.ietf.org/html/rfc7519#section-4.1.6 "iat" and other time fields must contain
// number of seconds from 1970-01-01T00:00:00Z UTC until the specified UTC date/time.
// The https://github.com/dgrijalva/jwt-go marshals time as non integer.
return token.SignedString(settings.ServerSignature, jwt.WithMarshaller(func(ctx jwt.CodingContext, v interface{}) ([]byte, error) {
if std, ok := v.(jwt.StandardClaims); ok {
return json.Marshal(standardClaims{
Audience: std.Audience,
ExpiresAt: unixTimeOrZero(std.ExpiresAt),
ID: std.ID,
IssuedAt: unixTimeOrZero(std.IssuedAt),
Issuer: std.Issuer,
NotBefore: unixTimeOrZero(std.NotBefore),
Subject: std.Subject,
})
}
return json.Marshal(v)
}))
return token.SignedString(settings.ServerSignature)
}

// GetSubjectAccountAndCapability analyzes Argo CD account token subject and extract account name
Expand Down Expand Up @@ -499,10 +473,8 @@ func (mgr *SessionManager) VerifyUsernamePassword(username string, password stri
// VerifyToken verifies if a token is correct. Tokens can be issued either from us or by an IDP.
// We choose how to verify based on the issuer.
func (mgr *SessionManager) VerifyToken(tokenString string) (jwt.Claims, string, error) {
parser := &jwt.Parser{
ValidationHelper: jwt.NewValidationHelper(jwt.WithoutClaimsValidation(), jwt.WithoutAudienceValidation()),
}
var claims jwt.StandardClaims
parser := jwt.NewParser(jwt.WithoutClaimsValidation())
var claims jwt.RegisteredClaims
_, _, err := parser.ParseUnverified(tokenString, &claims)
if err != nil {
return nil, "", err
Expand Down
2 changes: 1 addition & 1 deletion util/session/sessionmanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"testing"
"time"

"github.com/dgrijalva/jwt-go/v4"
"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
Expand Down