-
Notifications
You must be signed in to change notification settings - Fork 432
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
feat(api, cli, ui): auth consumer token expiration and last authentication #5822
Conversation
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
ee1d0de
to
639834c
Compare
Signed-off-by: francois samin <[email protected]>
Signed-off-by: francois samin <[email protected]>
cli/cdsctl/consumer.go
Outdated
@@ -86,6 +87,9 @@ var authConsumerNewCmd = cli.Command{ | |||
Name: "scopes", | |||
Type: cli.FlagSlice, | |||
Usage: "Define the list of scopes for the consumer", | |||
}, { | |||
Name: "duration", | |||
Usage: "Validity period of the token generated for the consumer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usage: "Validity period of the token generated for the consumer", | |
Usage: "Validity period of the token generated for the consumer (in days)", |
req.NewDuration = api.Config.Auth.TokenDefaultDuration | ||
} | ||
var overlapDuration time.Duration | ||
if req.OverlapDuration != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should check that overlap duration is less than duration ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
var overlapDuration time.Duration | ||
if req.OverlapDuration != "" { | ||
overlapDuration, err = time.ParseDuration(req.OverlapDuration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not both overlap and validity duration in same format (string duration vs count of hours) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because overlap should be in minutes or hours, and validity should be a matter of days
@@ -24,7 +24,8 @@ func NewConsumerWorker(ctx context.Context, db gorpmapper.SqlExecutorWithTx, nam | |||
sdk.AuthConsumerScopeRunExecution, | |||
sdk.AuthConsumerScopeService, | |||
), | |||
IssuedAt: time.Now(), | |||
//IssuedAt: time.Now(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To remove ?
@@ -46,7 +47,8 @@ func NewConsumerExternal(ctx context.Context, db gorpmapper.SqlExecutorWithTx, u | |||
"username": userInfo.Username, | |||
"email": userInfo.Email, | |||
}, | |||
IssuedAt: time.Now(), | |||
//IssuedAt: time.Now(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To remove ?
@@ -127,6 +128,7 @@ func InsertConsumer(ctx context.Context, db gorpmapper.SqlExecutorWithTx, ac *sd | |||
|
|||
// UpdateConsumer in database. | |||
func UpdateConsumer(ctx context.Context, db gorpmapper.SqlExecutorWithTx, ac *sdk.AuthConsumer) error { | |||
ac.ValidityPeriods.Sort() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be nice to sort also on get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
func checkSigninConsumerTokenIssuedAt(ctx context.Context, payload signinBuiltinConsumerToken, v sdk.AuthConsumerValidityPeriod) (string, error) { | ||
var eqIAT = payload.IAT == v.IssuedAt.Unix() | ||
var hasRevoke = v.Duration > 0 | ||
var beforeRevoke = time.Now().Before(v.IssuedAt.Add(v.Duration)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe only one afterRevoke value should be clearer.
} | ||
return authentication.SignJWS(payload, 0) // 0 means no expiration time | ||
return authentication.SignJWS(payload, latestValidityPeriod.Duration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add issued at value as parameter instead of using time.Now inside SignJWS func
engine/api/auth_local.go
Outdated
@@ -150,7 +150,7 @@ func initBuiltinConsumersFromStartupConfig(ctx context.Context, tx gorpmapper.Sq | |||
Data: map[string]string{}, | |||
GroupIDs: []int64{group.SharedInfraGroup.ID}, | |||
ScopeDetails: scopes, | |||
IssuedAt: time.Unix(startupConfig.IAT, 0), | |||
ValidityPeriods: sdk.NewAuthConsumerValidityPeriod(time.Unix(startupConfig.IAT, 0), 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set a value for services consumers validity period that can be greater than other consumers. Maybe we should update doc for services setup where 'consumer new' command is used.
Signed-off-by: francois samin <[email protected]>
CDS Report build-all-cds#16086.0 ✘
|
Signed-off-by: francois samin <[email protected]>
CDS Report build-all-cds#16108.0 ✘
|
CDS Report build-all-cds#16108.1 ✘
|
CDS Report build-all-cds#16108.2 ✘
|
CDS Report build-all-cds#16108.3 ✘
|
Signed-off-by: francois samin <[email protected]>
CDS Report build-all-cds#16141.0 ✘
|
SonarCloud Quality Gate failed. |
No description provided.