feat(Session cookie support for Auth to Admin Server) #545

rajkong · 2021-12-10T18:27:27Z

Adding support for session cookie file in Netscape cookie file format to use in cookie jar. Depending on the User Agent that was used to generate the cookie file, the User-Agent header in deck may need to be over-ridden

hbagdi · 2021-12-10T19:38:23Z

utils/cookiejarparser.go

@@ -0,0 +1,114 @@
+package utils
+
+// Code from https://github.com/ssgelm/cookiejarparser under MIT License


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

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

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).

hbagdi · 2021-12-10T19:39:05Z

utils/types.go

@@ -94,6 +94,11 @@ type KongClientConfig struct {
 	HTTPClient *http.Client

 	Timeout int
+
+	SessionFilePath string


Suggested change

SessionFilePath string

CookieJarPath string

Cookies can do more than sessions.

.gitignore

hbagdi · 2021-12-10T19:40:39Z

cmd/root.go

@@ -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", "",


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.

hbagdi · 2021-12-10T19:41:33Z

cmd/root.go

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


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?

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

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.

hbagdi · 2021-12-10T19:42:22Z

utils/types.go

+	SessionFilePath string
+
+	// Whether to initialize the Http client with a cookie jar or not
+	ISSessionClient bool


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

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

codecov-commenter · 2021-12-10T22:36:38Z

Codecov Report

Merging #545 (4b4bc86) into main (f834d26) will decrease coverage by 0.01%.
The diff coverage is 43.75%.

@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
- Coverage   51.98%   51.96%   -0.02%     
==========================================
  Files          73       73              
  Lines        7714     7730      +16     
==========================================
+ Hits         4010     4017       +7     
- Misses       3354     3363       +9     
  Partials      350      350

Impacted Files	Coverage Δ
utils/types.go	`23.80% <0.00%> (-1.45%)`	⬇️
cmd/root.go	`55.78% <70.00%> (+0.78%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f834d26...4b4bc86. Read the comment docs.

hbagdi · 2021-12-13T18:05:14Z

cmd/root.go

@@ -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-cookie-jar-path", "",
+		"Absolute path for cookie-jar file in Netscape file format for auth with Admin Server.\n"+


Suggested change

"Absolute path for cookie-jar file in Netscape file format for auth with Admin Server.\n"+

"Absolute path to a cookie-jar file in the Netscape cookie format for auth with Admin Server.\n"+

hbagdi · 2021-12-13T18:06:06Z

utils/types.go

+	if opt.CookieJarPath != "" {
+		jar, err := LoadCookieJarFile(opt.CookieJarPath)
+		if err != nil {
+			return nil, fmt.Errorf("failed to initialize cookie-jar:%w", err)


Suggested change

return nil, fmt.Errorf("failed to initialize cookie-jar:%w", err)

return nil, fmt.Errorf("failed to initialize cookie-jar: %w", err)

# Conflicts: # go.mod # go.sum

From #545

rajkong added 3 commits December 10, 2021 09:55

First working version

0097bb7

Formatting etc

30145fa

Lint]

65f88e4

rajkong requested a review from a team as a code owner December 10, 2021 18:27

rajkong changed the title ~~Session cookie support for Auth to Admin Server~~ feat(Session cookie support for Auth to Admin Server) Dec 10, 2021

hbagdi reviewed Dec 10, 2021

View reviewed changes

rajkong added 3 commits December 10, 2021 14:24

Address review comments

7da551c

Reverted the checking in of idea files

5337fa3

Formatting

9c30f51

Removing session termwq

e027741

hbagdi reviewed Dec 13, 2021

View reviewed changes

rajkong added 5 commits December 13, 2021 14:30

cookieparsers code as external dep

b50259e

Merge branch 'main' into session-cookie

a0a2e5c

# Conflicts: # go.mod # go.sum

Fixing cookieparser go.sum

6925ea5

Fixing language

1f29a42

Deleted test file

4b4bc86

hbagdi approved these changes Dec 14, 2021

View reviewed changes

hbagdi merged commit 41689ef into Kong:main Dec 14, 2021

rainest mentioned this pull request Dec 14, 2021

feat (BasicAuthSession support for Kong Admin APIs) #537

Closed

AntoineJac pushed a commit that referenced this pull request Jan 23, 2024

feat: cookie support for auth purposes to Admin Server

4a56bca

From #545

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Session cookie support for Auth to Admin Server) #545

feat(Session cookie support for Auth to Admin Server) #545

rajkong commented Dec 10, 2021

hbagdi Dec 10, 2021

rajkong Dec 10, 2021

hbagdi Dec 13, 2021

rajkong Dec 13, 2021

hbagdi Dec 10, 2021

rajkong Dec 13, 2021

hbagdi Dec 10, 2021

rajkong Dec 10, 2021

hbagdi Dec 10, 2021

rajkong Dec 10, 2021

hbagdi Dec 13, 2021

hbagdi Dec 10, 2021

rajkong Dec 10, 2021

rajkong Dec 10, 2021

codecov-commenter commented Dec 10, 2021 •

edited

Loading

hbagdi Dec 13, 2021

rajkong Dec 14, 2021

hbagdi Dec 13, 2021

rajkong Dec 14, 2021

		@@ -0,0 +1,114 @@
		package utils

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

	rootCmd.PersistentFlags().String("kong-session-cookie-path", "",
	rootCmd.PersistentFlags().String("kong-cookie-jar-path", "",

	"Absolute path for cookie-jar file in Netscape file format for auth with Admin Server.\n"+
	"Absolute path to a cookie-jar file in the Netscape cookie format for auth with Admin Server.\n"+

	return nil, fmt.Errorf("failed to initialize cookie-jar:%w", err)
	return nil, fmt.Errorf("failed to initialize cookie-jar: %w", err)

feat(Session cookie support for Auth to Admin Server) #545

feat(Session cookie support for Auth to Admin Server) #545

Conversation

rajkong commented Dec 10, 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

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

codecov-commenter commented Dec 10, 2021 • edited Loading

Codecov Report

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

codecov-commenter commented Dec 10, 2021 •

edited

Loading