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.

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(Session cookie support for Auth to Admin Server) #545

Merged
merged 12 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ _testmain.go
*.test
*.prof

#IDE ignore
.idea

rajkong marked this conversation as resolved.
Show resolved Hide resolved
# for this repo only
deck
dist/
Expand Down
14 changes: 14 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ func init() {
viper.BindPFlag("skip-workspace-crud",
rootCmd.PersistentFlags().Lookup("skip-workspace-crud"))

// Support for Session Cookie
rootCmd.PersistentFlags().String("kong-session-cookie-path", "",
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
rootCmd.PersistentFlags().String("kong-session-cookie-path", "",
rootCmd.PersistentFlags().String("kong-cookie-jar-path", "",

Let's avoid the word 'session' in here since the functionality is more generic than that.

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

"Absolute path for the session-cookie file for auth with Admin Server.\n"+
"You may also need to pass in as header the User-Agent that was used to create the session.")
Copy link
Member

Choose a reason for hiding this comment

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

Update this text as well.
Please mention the format of the cookie file path (netscape cookie format I think).

Question: what happens if the user-agent is not specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gives a 401. This took me a while to figure out. Looks like Kong associates sessions and user-agent. My guess is that when Kong sees a different user agent, it assumes that the session was hijacked and sends a 401

Copy link
Member

Choose a reason for hiding this comment

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

Yikes. That may introduce additional friction that we can't solve for. Let's ship this and come back here if this PR doesn't achieve the desired outcome for our customers.

viper.BindPFlag("kong-session-cookie-path",
rootCmd.PersistentFlags().Lookup("kong-session-cookie-path"))

// konnect-specific flags
rootCmd.PersistentFlags().String("konnect-email", "",
"Email address associated with your Konnect account.")
Expand Down Expand Up @@ -207,6 +214,13 @@ func initConfig() {
rootConfig.Debug = (viper.GetInt("verbose") >= 1)
rootConfig.Timeout = (viper.GetInt("timeout"))

// Session cookie support
rootConfig.SessionFilePath = viper.GetString("kong-session-cookie-path")
if rootConfig.SessionFilePath != "" {
rootConfig.ISSessionClient = true
} else {
rootConfig.ISSessionClient = false
}
color.NoColor = (color.NoColor || viper.GetBool("no-color"))

if err := initKonnectConfig(); err != nil {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ require (
github.com/yudai/gojsondiff v1.0.0
github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 // indirect
github.com/yudai/pp v2.0.1+incompatible // indirect
golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 // indirect
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
k8s.io/code-generator v0.22.4
)
114 changes: 114 additions & 0 deletions utils/cookiejarparser.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package utils

// Code from https://github.com/ssgelm/cookiejarparser under MIT License
Copy link
Member

Choose a reason for hiding this comment

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

Any reason we are copying this code rather than using it as a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seemed very small and not sure if it would be maintained. So felt it is better to copy instead of import

Copy link
Member

Choose a reason for hiding this comment

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

If we are not modifying any part of the dependency code, let's introduce the dependency.
Copying code in here is a new pattern and I'd recommend not deviating from current practices (other and future maintainers will wonder why this exception).

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

import (
"bufio"
"fmt"
"net/http"
"net/http/cookiejar"
"net/url"
"os"
"strconv"
"strings"
"time"

"golang.org/x/net/publicsuffix"
)

const (
httpOnlyPrefix = "#HttpOnly_"
maxEntriesLen = 7
)

func parseCookieLine(cookieLine string, lineNum int) (*http.Cookie, error) {
var err error
cookieLineHTTPOnly := false
if strings.HasPrefix(cookieLine, httpOnlyPrefix) {
cookieLineHTTPOnly = true
cookieLine = strings.TrimPrefix(cookieLine, httpOnlyPrefix)
}

if strings.HasPrefix(cookieLine, "#") || cookieLine == "" {
return nil, nil
}

cookieFields := strings.Split(cookieLine, "\t")

if len(cookieFields) < 6 || len(cookieFields) > 7 {
return nil, fmt.Errorf("incorrect number of fields in line %d. Expected 6 or 7, got %d", lineNum, len(cookieFields))
}

for i, v := range cookieFields {
cookieFields[i] = strings.TrimSpace(v)
}

cookie := &http.Cookie{
Domain: cookieFields[0],
Path: cookieFields[2],
Name: cookieFields[5],
HttpOnly: cookieLineHTTPOnly,
}
cookie.Secure, err = strconv.ParseBool(cookieFields[3])
if err != nil {
return nil, err
}
expiresInt, err := strconv.ParseInt(cookieFields[4], 10, 64)
if err != nil {
return nil, err
}
if expiresInt > 0 {
cookie.Expires = time.Unix(expiresInt, 0)
}

if len(cookieFields) == maxEntriesLen {
cookie.Value = cookieFields[6]
}

return cookie, nil
}

// LoadCookieJarFile takes a path to a curl (netscape) cookie jar file and crates a go http.CookieJar with the contents
func LoadCookieJarFile(path string) (http.CookieJar, error) {
jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
if err != nil {
return nil, err
}

file, err := os.Open(path)
if err != nil {
return nil, err
}
defer file.Close()

lineNum := 1
scanner := bufio.NewScanner(file)
for scanner.Scan() {
cookieLine := scanner.Text()
cookie, err := parseCookieLine(cookieLine, lineNum)
if cookie == nil {
continue
}
if err != nil {
return nil, err
}

var cookieScheme string
if cookie.Secure {
cookieScheme = "https"
} else {
cookieScheme = "http"
}
cookieURL := &url.URL{
Scheme: cookieScheme,
Host: cookie.Domain,
}

cookies := jar.Cookies(cookieURL)
cookies = append(cookies, cookie)
jar.SetCookies(cookieURL, cookies)

lineNum++
}

return jar, nil
}
105 changes: 105 additions & 0 deletions utils/cookiejarparser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package utils

// Code from https://github.com/ssgelm/cookiejarparser under MIT License

import (
"encoding/json"
"net/http"
"net/http/cookiejar"
"net/url"
"reflect"
"testing"

"github.com/stretchr/testify/require"
"golang.org/x/net/publicsuffix"
)

func TestParseCookieLine(t *testing.T) {
// normal
cookie, err := parseCookieLine("example.com FALSE / FALSE 0 test_cookie 1", 1)
sampleCookie := &http.Cookie{
Domain: "example.com",
Path: "/",
Name: "test_cookie",
Value: "1",
HttpOnly: false,
Secure: false,
}

if !reflect.DeepEqual(cookie, sampleCookie) || err != nil {
c1, _ := json.Marshal(cookie)
c2, _ := json.Marshal(sampleCookie)

t.Errorf("Parsing normal cookie failed. Expected:\n "+
"cookie: %s err: nil,\ngot:\n cookie: %s err: %s", c2, c1, err)
}
// httponly
cookieHTTP, err := parseCookieLine("#HttpOnly_example.com FALSE / FALSE 0 test_cookie_httponly 1", 1)
sampleCookieHTTP := &http.Cookie{
Domain: "example.com",
Path: "/",
Name: "test_cookie_httponly",
Value: "1",
HttpOnly: true,
Secure: false,
}

if !reflect.DeepEqual(cookieHTTP, sampleCookieHTTP) || err != nil {
c1, _ := json.Marshal(cookieHTTP)
c2, _ := json.Marshal(sampleCookieHTTP)

t.Errorf("Parsing httpOnly cookie failed. Expected:\n "+
"cookie: %s err: nil,\ngot:\n cookie: %s err: %s", c2, c1, err)
}

// comment
cookieComment, err := parseCookieLine("# This is a comment", 1)
if cookieComment != nil || err != nil {
t.Errorf("Parsing comment failed. Expected cookie: nil err: nil, "+
"got cookie: %s err: %s", cookie, err)
}

cookieBlank, err := parseCookieLine("", 1)
if cookieBlank != nil || err != nil {
t.Errorf("Parsing blank line failed. Expected cookie: nil err: nil, "+
"got cookie: %s err: %s", cookie, err)
}
}

func TestLoadCookieJarFile(t *testing.T) {
exampleURL := &url.URL{
Scheme: "http",
Host: "example.com",
}
sampleCookies := []*http.Cookie{
{
Domain: "example.com",
Path: "/",
Name: "test_cookie",
Value: "1",
HttpOnly: false,
Secure: false,
},
{
Domain: "example.com",
Path: "/",
Name: "test_cookie_httponly",
Value: "1",
HttpOnly: true,
Secure: false,
},
}
sampleCookieJar, _ := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
sampleCookieJar.SetCookies(exampleURL, sampleCookies)

cookieJar, err := LoadCookieJarFile("testdata/cookies.txt")
require.NoError(t, err)

c1, _ := json.Marshal(cookieJar.Cookies(exampleURL))
c2, _ := json.Marshal(sampleCookieJar.Cookies(exampleURL))

if !reflect.DeepEqual(c1, c2) || err != nil {
t.Errorf("Cookie jar creation failed. Expected:\n "+
"cookieJar: %s err: nil,\ngot:\n cookieJar: %s err: %s", c2, c1, err)
}
}
4 changes: 4 additions & 0 deletions utils/testdata/cookies.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Test cookie file

example.com FALSE / FALSE 0 test_cookie 1
#HttpOnly_example.com FALSE / FALSE 0 test_cookie_httponly 1
13 changes: 13 additions & 0 deletions utils/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ type KongClientConfig struct {
HTTPClient *http.Client

Timeout int

SessionFilePath string
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
SessionFilePath string
CookieJarPath string

Cookies can do more than sessions.

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


// Whether to initialize the Http client with a cookie jar or not
ISSessionClient bool
Copy link
Member

Choose a reason for hiding this comment

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

Is this required? Why not check if the SessionFilePath != ""?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I could do that, felt that the boolean could satisfy multiple flags, so left it. I can remove it

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

}

type KonnectConfig struct {
Expand Down Expand Up @@ -153,6 +158,14 @@ func GetKongClient(opt KongClientConfig) (*kong.Client, error) {
if opt.Workspace != "" {
url.Path = path.Join(url.Path, opt.Workspace)
}
// Add Session Cookie support if required
if opt.ISSessionClient {
jar, err := LoadCookieJarFile(opt.SessionFilePath)
if err != nil {
return nil, fmt.Errorf("failed to initialize session jar:%w", err)
}
c.Jar = jar
}

kongClient, err := kong.NewClient(kong.String(url.String()), c)
if err != nil {
Expand Down