Skip to content

Conversation

@bochunz
Copy link
Contributor

@bochunz bochunz commented Aug 26, 2016

Tests pending

To try it:
export GOPATH=[path/to]/oauth2l/oauth2l-go
go run [path/to]/oauth2l-go/src/oauth2l/oauth2l.go --json <secret.json> fetch cloud-platform

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e88bfcb on bochunz:go into 8c93215 on google:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 66f05ad on bochunz:go into 8c93215 on google:master.

return token
}

// Get an OAuth2 token given a client secret json or service account json.
Copy link
Contributor

Choose a reason for hiding this comment

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

Get an OAuth 2 access token for the specified OAuth client info and scopes.
clientInfo: a JSON text that represents either an OAuth client ID or a service account.
scopes: a list of OAuth scopes. If the scope is not a normal URL, it will be prefixed with xxx.
return: the return value will be xxx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@wora
Copy link
Contributor

wora commented Aug 27, 2016

Great job.

Initial set of comments. After we are done, let's ask Olson to comment on Go style.

@wora
Copy link
Contributor

wora commented Aug 29, 2016

Have you submitted the new changes?

@bochunz
Copy link
Contributor Author

bochunz commented Aug 29, 2016

Not yet, fixing some style issue

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f51fc48 on bochunz:go into 8c93215 on google:master.

@bochunz
Copy link
Contributor Author

bochunz commented Aug 29, 2016

Uploaded new chage

func defaultAuthorizeFlowHandler(authorizeUrl string) string {
// Print the url on console, let user authorize and paste the token back.
fmt.Printf("Go to the following link in your browser:\n\n %s\n\n",
authorizeUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember Go doesn't have line length limit. Why do we need to wrap the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed line break

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5bc5693 on bochunz:go into 8c93215 on google:master.

}

// Run the three-legged oauth authorize flow.
func authorizeFlow(secret map[string]interface{}, scopes string, handler func(string) string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use "scope string" everywhere in this file, because that is OAuth 2 spec. The scopes []string should only be used in the main.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@wora
Copy link
Contributor

wora commented Aug 30, 2016

I think we are almost there.

@bochunz bochunz force-pushed the go branch 2 times, most recently from b7e0788 to cd9a979 Compare August 30, 2016 20:43
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b7e0788 on bochunz:go into 8c93215 on google:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling cd9a979 on bochunz:go into 8c93215 on google:master.

fmt.Printf("Go to the following link in your browser:\n\n %s\n\n", authorizeUrl)

fmt.Println("Enter verification code: ")
code := ""

Choose a reason for hiding this comment

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

var code string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@wora
Copy link
Contributor

wora commented Aug 30, 2016

LGTM

Please return original error as much as possible.

@wora
Copy link
Contributor

wora commented Aug 31, 2016

LGTM

Well done.

@bochunz bochunz changed the title Add oauth2-go Add oauth2l-go Aug 31, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8347a32 on bochunz:go into 8c93215 on google:master.

@bochunz bochunz merged commit d3dfd93 into google:master Aug 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants