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

Jump to bottom

76 google oauth config mutator #77

Merged
merged 8 commits into from
Jan 23, 2024
Merged

76 google oauth config mutator #77

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6429088
Add a method to authenticate without a config file
calston Apr 11, 2019
3c2d74d
Redirect to login page on auth failure
calston Apr 11, 2019
78c98f3
Modify setup from stirng and include test.
chrishalbert Jan 16, 2024
520614c
Add with login url.
chrishalbert Jan 16, 2024
0d3b4b8
Godocicize comment.
chrishalbert Jan 16, 2024
f83e7e1
Add light url validation and sanitization with tests.
chrishalbert Jan 20, 2024
653d40b
Fix description
chrishalbert Jan 20, 2024
634cbb0
Fix linting issue.
chrishalbert Jan 20, 2024
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
60 changes: 42 additions & 18 deletions google/google.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"encoding/gob"
"encoding/json"
"fmt"

"net/http"
"os"

Expand Down Expand Up @@ -44,6 +45,8 @@ func init() {
gob.Register(goauth.Userinfo{})
}

var loginURL string
fogfish marked this conversation as resolved.
Show resolved Hide resolved

func randToken() string {
b := make([]byte, 32)
if _, err := rand.Read(b); err != nil {
Expand Down Expand Up @@ -74,6 +77,19 @@ func Setup(redirectURL, credFile string, scopes []string, secret []byte) {
}
}

// SetupFromString accepts string values for ouath2 Configs
func SetupFromString(redirectURL, clientID string, clientSecret string, scopes []string, secret []byte) {
store = cookie.NewStore(secret)

conf = &oauth2.Config{
ClientID: clientID,
ClientSecret: clientSecret,
RedirectURL: redirectURL,
Scopes: scopes,
Endpoint: google.Endpoint,
}
}

func Session(name string) gin.HandlerFunc {
return sessions.Sessions(name, store)
}
Expand All @@ -100,29 +116,33 @@ func GetLoginURL(state string) string {
return conf.AuthCodeURL(state)
}

func WithLoginURL(url string) {
loginURL = url
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
loginURL = url
loginURL = strings.TrimSpace(s)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe better to use url.Parse(s), check error and if no error use u.String() to have a clean url 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.

With the error handling, would you suggest the signature change to func WithLoginURL(url string) (bool, error) ?

Copy link
Member

Choose a reason for hiding this comment

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

I would say return error is enough. Either there is an error or not :)

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest we are aiming for consistency on API. It is either

func SetupFromString(redirectURL, clientID string, clientSecret string, scopes []string, secret []byte) error
func WithLoginURL(url string) error

or

func SetupFromString(redirectURL, clientID string, clientSecret string, scopes []string, secret []byte)
func WithLoginURL(url string)

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to go with the first, so return error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense - that seems more idiomatic too. I'll have that shortly..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay - had some distractions. I was just getting ready to update the PR and noticed that SetupFromString() and Setup() would differ with regards to their return value. The Setup() function could more plausibly have errors, however, errors aren't being returned at this time. I do like the idea of returning an error WithLoginURL() since there is possibility for a bad URL being added, but I feel like if I return an error from SetupFromString(), I should also return an error for Setup(). While that wouldn't break functionality, it could cause linting errors since return values aren't being accepted and handled. I'm going to make the changes for WithLoginURL() as prescribed, but I'm interested to get your opinions on handling SetupFromString() and Setup().

}

// Auth is the google authorization middleware. You can use them to protect a routergroup.
// Example:
//
// private.Use(google.Auth())
// private.GET("/", UserInfoHandler)
// private.GET("/api", func(ctx *gin.Context) {
// ctx.JSON(200, gin.H{"message": "Hello from private for groups"})
// })
// private.Use(google.Auth())
// private.GET("/", UserInfoHandler)
// private.GET("/api", func(ctx *gin.Context) {
// ctx.JSON(200, gin.H{"message": "Hello from private for groups"})
// })
//
// // Requires google oauth pkg to be imported as `goauth "google.golang.org/api/oauth2/v2"`
// func UserInfoHandler(ctx *gin.Context) {
// var (
// res goauth.Userinfo
// ok bool
// )
// // Requires google oauth pkg to be imported as `goauth "google.golang.org/api/oauth2/v2"`
// func UserInfoHandler(ctx *gin.Context) {
// var (
// res goauth.Userinfo
// ok bool
// )
//
// val := ctx.MustGet("user")
// if res, ok = val.(goauth.Userinfo); !ok {
// res = goauth.Userinfo{Name: "no user"}
// }
// val := ctx.MustGet("user")
// if res, ok = val.(goauth.Userinfo); !ok {
// res = goauth.Userinfo{Name: "no user"}
// }
//
// ctx.JSON(http.StatusOK, gin.H{"Hello": "from private", "user": res.Email})
// }
// ctx.JSON(http.StatusOK, gin.H{"Hello": "from private", "user": res.Email})
// }
func Auth() gin.HandlerFunc {
return func(ctx *gin.Context) {
// Handle the exchange code to initiate a transport.
Expand All @@ -137,7 +157,11 @@ func Auth() gin.HandlerFunc {

retrievedState := session.Get(stateKey)
if retrievedState != ctx.Query(stateKey) {
ctx.AbortWithError(http.StatusUnauthorized, fmt.Errorf("invalid session state: %s", retrievedState))
if loginURL != "" {
ctx.Redirect(302, loginURL)
} else {
ctx.AbortWithError(http.StatusUnauthorized, fmt.Errorf("invalid session state: %s", retrievedState))
}
return
}

Expand Down
29 changes: 29 additions & 0 deletions google/google_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package google

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestSetupFromString(t *testing.T) {
t.Run("should assign store and config accordingly", func(t *testing.T) {
store = nil
conf = nil
SetupFromString("http://fake.fake", "clientid", "clientsecret", []string{}, []byte("secret"))
assert.NotNil(t, conf)
assert.NotNil(t, store)
assert.Equal(t, conf.ClientID, "clientid")
assert.Equal(t, conf.ClientSecret, "clientsecret")
})
}

func TestWithLoginURL(t *testing.T) {
t.Run("should assign the login url", func(t *testing.T) {
loginURL = ""
url := "http://fake.fake"
WithLoginURL(url)
assert.NotEmpty(t, url)
assert.Equal(t, url, loginURL)
})
}