API to refresh access token api for user #136

sm43 · 2020-11-18T18:41:09Z

This adds implementation for API to refresh access token for user. This
requires user refresh token to be passed to get a new access token.

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.

PuneetPunamiya · 2021-02-16T09:51:52Z

api/design/auser.go

@@ -0,0 +1,57 @@
+// Copyright © 2020 The Tekton Authors.


Should it be 2021 ??

PuneetPunamiya · 2021-02-16T09:59:44Z

api/design/auser.go

+			Header("refreshToken:Authorization")
+
+			Response(StatusOK)
+			Response("internal-error", StatusInternalServerError)


Are we missing invalid authorization code error ??

why do we need invalid authorization code error ? this is not login api

Then why are we mentioning it above ??

ohh.. willl remove it :P

PuneetPunamiya · 2021-02-16T10:13:02Z

api/pkg/service/user/user.go

@@ -0,0 +1,130 @@
+// Copyright © 2020 The Tekton Authors.


Same as above

sm43 · 2021-02-16T10:16:51Z

api/design/auser.go

+var _ = Service("user", func() {
+	Description("The user service exposes endpoint to get user specific specs")
+
+	Error("invalid-code", ErrorResult, "Invalid Authorization code")


This adds an API to refresh access token for user. This requires user refresh token to be passed to get a new access token. Signed-off-by: Shivam Mukhade <[email protected]>

pratap0007 · 2021-02-23T04:36:47Z

/lgtm

PuneetPunamiya · 2021-02-23T08:41:40Z

api/pkg/service/user/user.go

+	return userScopes, nil
+}
+
+func createChecksum(token string) string {


Can you please add a unit test for this

Do we need a separate test? this is being tested with the API as we store it in db and check the db record.

In db we just check if we have checksum, what I think is passing the token it should return the checksum is what this function does and hence I think we need to test it

yeah but before checking in db it creates the checksum

PuneetPunamiya · 2021-02-23T08:45:42Z

api/design/user.go

+	Description("The user service exposes endpoint to get user specific specs")
+
+	Error("invalid-token", ErrorResult, "Invalid User token")
+	Error("invalid-scopes", ErrorResult, "Invalid User scope")


I don't think we have used this error in the implementation of the api

It is being by the auth service while verifying jwt

piyush-garg · 2021-02-24T03:01:16Z

/lgtm

PuneetPunamiya · 2021-02-24T04:14:28Z

/approve
/meow

tekton-robot · 2021-02-24T04:14:29Z

@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 · 2021-02-24T04:14:31Z

[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

PuneetPunamiya · 2021-02-24T04:52:00Z

/woof

tekton-robot · 2021-02-24T04:52:01Z

@PuneetPunamiya:

In response to this:

/woof

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 requested review from pratap0007 and vdemeester November 18, 2020 18:41

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 18, 2020

sm43 changed the title ~~Api to refresh access token api~~ API to refresh access token api for user 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

sm43 force-pushed the refresh-access-token-api branch from e4d43df to 1913cf5 Compare December 10, 2020 17:32

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 added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 24, 2020

sm43 force-pushed the refresh-access-token-api branch from 1913cf5 to e0e89ad Compare January 4, 2021 05:00

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 4, 2021

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2021

sm43 force-pushed the refresh-access-token-api branch from e0e89ad to abbb243 Compare January 25, 2021 04:49

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2021

sm43 force-pushed the refresh-access-token-api branch from abbb243 to 8d0e48c Compare January 25, 2021 04:54

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2021

sm43 force-pushed the refresh-access-token-api branch from 8d0e48c to 2143c2a Compare January 25, 2021 05:26

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2021

PuneetPunamiya reviewed Feb 16, 2021

View reviewed changes

api/pkg/service/user/user.go Outdated

@@ -0,0 +1,130 @@

// Copyright © 2020 The Tekton Authors.

Copy link

Member

PuneetPunamiya Feb 16, 2021

Choose a reason for hiding this comment

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

Same as above

sm43 commented Feb 16, 2021

View reviewed changes

sm43 force-pushed the refresh-access-token-api branch 2 times, most recently from 9a1c3ad to a05a0f0 Compare February 22, 2021 10:10

Adds an API to refresh User access token

29a4462

This adds an API to refresh access token for user. This requires user refresh token to be passed to get a new access token. Signed-off-by: Shivam Mukhade <[email protected]>

sm43 force-pushed the refresh-access-token-api branch from a05a0f0 to 29a4462 Compare February 22, 2021 12:23

tekton-robot assigned pratap0007 Feb 23, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2021

PuneetPunamiya reviewed Feb 23, 2021

View reviewed changes

tekton-robot assigned piyush-garg Feb 24, 2021

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2021

tekton-robot merged commit 2e33359 into tektoncd:master Feb 24, 2021

sm43 deleted the refresh-access-token-api branch April 15, 2021 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API to refresh access token api for user #136

API to refresh access token api for user #136

sm43 commented Nov 18, 2020 •

edited

Loading

PuneetPunamiya Feb 16, 2021

sm43 Feb 16, 2021

PuneetPunamiya Feb 16, 2021

sm43 Feb 16, 2021

PuneetPunamiya Feb 16, 2021

sm43 Feb 16, 2021

PuneetPunamiya Feb 16, 2021

sm43 Feb 16, 2021

pratap0007 commented Feb 23, 2021

PuneetPunamiya Feb 23, 2021

sm43 Feb 23, 2021

PuneetPunamiya Feb 23, 2021

sm43 Feb 23, 2021

PuneetPunamiya Feb 23, 2021

sm43 Feb 23, 2021

piyush-garg commented Feb 24, 2021

PuneetPunamiya commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

PuneetPunamiya commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

API to refresh access token api for user #136

API to refresh access token api for user #136

Conversation

sm43 commented Nov 18, 2020 • edited Loading

Submitter Checklist

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pratap0007 commented Feb 23, 2021

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

piyush-garg commented Feb 24, 2021

PuneetPunamiya commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

PuneetPunamiya commented Feb 24, 2021

tekton-robot commented Feb 24, 2021

sm43 commented Nov 18, 2020 •

edited

Loading