Updates Auth API to return Access and Refresh Token #128

sm43 · 2020-10-27T19:18:47Z

This adds expiry time in user jwt and auth returns refresh jwt
along with access jwt. The refresh jwt can be later used to get a
new access jwt.

Signed-off-by: Shivam Mukhade smukhade@redhat.com

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Commit messages follow commit message best practices

See the contribution guide for more details.

sm43 · 2020-10-27T19:22:38Z

/hold
till preview UI is updated

PuneetPunamiya · 2020-10-29T04:52:13Z

api/pkg/db/migration/202010241300_create_user_token_table.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package migration


Just a question do we need to add a test for this ?

not sure about this

piyush-garg · 2020-11-09T02:39:02Z

api/pkg/app/app.go

@@ -60,15 +61,22 @@ type APIBase struct {
 type Config interface {
 	BaseConfig
 	OAuthConfig() *oauth2.Config
-	JWTSigningKey() string
+	JWT() *JWT
 }

 // APIConfig defines struct on top of APIBase with GitHub Oauth
 // Configuration & JWT Signing Key


JWT configuration

piyush-garg · 2020-11-09T02:45:34Z

/lgtm

sthaha · 2020-11-09T03:58:44Z

api/pkg/app/app.go

@@ -60,15 +61,22 @@ type APIBase struct {
 type Config interface {
 	BaseConfig
 	OAuthConfig() *oauth2.Config
-	JWTSigningKey() string
+	JWT() *JWT


could you please name this JWTConfig ? it isn't a jwt, is it ?

sthaha · 2020-11-09T03:59:28Z

api/pkg/app/app.go

@@ -228,7 +236,7 @@ func FromEnvFile(file string) (*APIConfig, error) {
 	if ac.conf, err = initOAuthConfig(); err != nil {
 		return nil, err
 	}
-	if ac.jwtKey, err = jwtSigningKey(); err != nil {
+	if ac.jwt, err = jwtConfig(); err != nil {


ac.jwtConfig please :)

sthaha · 2020-11-09T04:00:39Z

api/pkg/app/app.go

+	return conf, nil
+}
+
+func computeDuration(dur string) (time.Duration, error) {


could you please consider writing a unit test for this ?

is this different from https://golang.org/pkg/time/#ParseDuration ?

sthaha · 2020-11-09T04:01:15Z

api/pkg/app/app.go

 // environment variables
-func jwtSigningKey() (string, error) {
+func jwtConfig() (*JWT, error) {


could you please consider writing a unit test ?

sthaha · 2020-11-09T04:04:46Z

api/pkg/app/app.go

+		return 0, fmt.Errorf("invalid duration value %s : %v. Valid format: '1d', '3h' or '15m'", dur, err)
+	}
+
+	switch format {


this wouldn't allow 1d12h45m10s would it ?

sm43 · 2020-11-11T16:27:44Z

/hold
as discussed we will merge this with UI's PR for auth flow.

This adds expiry time in user jwt and auth returns refresh jwt along with access jwt. The refresh jwt can be later used to get a new access jwt. Signed-off-by: Shivam Mukhade <smukhade@redhat.com>

sm43 · 2020-12-14T05:04:16Z

/retest

PuneetPunamiya · 2020-12-22T03:43:50Z

/lgtm

piyush-garg · 2020-12-23T08:30:16Z

/lgtm

PuneetPunamiya · 2020-12-23T08:41:20Z

/approve
/meow

tekton-robot · 2020-12-23T08:41:21Z

@PuneetPunamiya:

In response to this:

/approve
/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tekton-robot · 2020-12-23T08:41:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: PuneetPunamiya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [PuneetPunamiya]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot requested review from pratap0007 and vdemeester October 27, 2020 19:18

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 27, 2020

sm43 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 27, 2020

sm43 force-pushed the jwt-expiry branch 2 times, most recently from 3b3f5ec to 0bfc23d Compare October 28, 2020 17:38

PuneetPunamiya reviewed Oct 29, 2020

View reviewed changes

sm43 force-pushed the jwt-expiry branch 10 times, most recently from e79fdf4 to 9efbe8b Compare November 2, 2020 11:59

sm43 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2020

sm43 force-pushed the jwt-expiry branch from 9efbe8b to 0547332 Compare November 4, 2020 15:57

piyush-garg reviewed Nov 9, 2020

View reviewed changes

tekton-robot assigned piyush-garg Nov 9, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2020

sthaha reviewed Nov 9, 2020

View reviewed changes

sm43 force-pushed the jwt-expiry branch from 0547332 to 81ddd42 Compare November 9, 2020 09:25

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2020

sm43 force-pushed the jwt-expiry branch from 81ddd42 to 8483435 Compare November 9, 2020 09:36

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2020

sm43 force-pushed the jwt-expiry branch from 8483435 to 7a7b420 Compare November 18, 2020 16:45

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 18, 2020

sm43 force-pushed the jwt-expiry branch from 7a7b420 to 5970ddf Compare November 18, 2020 17:45

sm43 changed the title ~~Adds expiry time in User access JWT and auth also returns Refresh JWT~~ Updates Auth API to return Access and Refresh Token Nov 18, 2020

sm43 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2020

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 23, 2020

Adds expiry time in User access JWT and auth also returns Refresh JWT

16964e7

This adds expiry time in user jwt and auth returns refresh jwt along with access jwt. The refresh jwt can be later used to get a new access jwt. Signed-off-by: Shivam Mukhade <smukhade@redhat.com>

sm43 force-pushed the jwt-expiry branch from 5970ddf to 16964e7 Compare December 10, 2020 17:13

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 10, 2020

tekton-robot assigned PuneetPunamiya Dec 22, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2020

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 23, 2020

tekton-robot merged commit eae5fc9 into tektoncd:master Dec 23, 2020

sm43 deleted the jwt-expiry branch January 4, 2021 04:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates Auth API to return Access and Refresh Token #128

Updates Auth API to return Access and Refresh Token #128

sm43 commented Oct 27, 2020 •

edited

Loading

sm43 commented Oct 27, 2020 •

edited

Loading

PuneetPunamiya Oct 29, 2020

sm43 Oct 29, 2020

piyush-garg Nov 9, 2020

piyush-garg commented Nov 9, 2020

sthaha Nov 9, 2020

sthaha Nov 9, 2020

sthaha Nov 9, 2020

sthaha Nov 9, 2020

sthaha Nov 9, 2020

sthaha Nov 9, 2020

sm43 commented Nov 11, 2020

sm43 commented Dec 14, 2020

PuneetPunamiya commented Dec 22, 2020

piyush-garg commented Dec 23, 2020

PuneetPunamiya commented Dec 23, 2020

tekton-robot commented Dec 23, 2020

tekton-robot commented Dec 23, 2020

Updates Auth API to return Access and Refresh Token #128

Updates Auth API to return Access and Refresh Token #128

Conversation

sm43 commented Oct 27, 2020 • edited Loading

Submitter Checklist

sm43 commented Oct 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piyush-garg commented Nov 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm43 commented Nov 11, 2020

sm43 commented Dec 14, 2020

PuneetPunamiya commented Dec 22, 2020

piyush-garg commented Dec 23, 2020

PuneetPunamiya commented Dec 23, 2020

tekton-robot commented Dec 23, 2020

tekton-robot commented Dec 23, 2020

sm43 commented Oct 27, 2020 •

edited

Loading

sm43 commented Oct 27, 2020 •

edited

Loading