This repository has been archived by the owner on Jul 28, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 29
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use state parameter to encode the original visited URL and support fi…
…xed OIDC callback URL Use OIDC state parameter which is later returned during the auth flow to encode the original visited URL instead of using a cookie. Using a cookie is unnecessary in this case and even causes race conditions if multiple requests are made to a different OIDC-protected targets at the same time. Additionally, the previously-used cookie for this auth flow purpose was named the same as the session cookie, which could even effectively overwrite an existing valid session cookie. This commit also implements support for fixed callback URI/URLs as many public OIDC providers require admins to configure a fixed return/callback URLs in advance for every client to prevent abuse (e.g. by uploading a fake OIDC handler script on a different URI using some unprotected uploader and stealing tokens returned by the OIDC provider to this callback URL).
- Loading branch information
Mario Hros
committed
Feb 29, 2020
1 parent
ffc565b
commit 34fc892
Showing
16 changed files
with
414 additions
and
198 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "path" | ||
| "regexp" | ||
| "strings" | ||
| ) | ||
|
|
||
| var regexpURL = regexp.MustCompile(`(https?)://([^/]+)(/?.*)`) | ||
|
|
||
| func callbackDefinitionForClient(c Client) string { | ||
| callbackDef := "" | ||
|
|
||
| // also support empty clients (for tests without a valid client, using default callbackEndpoint) | ||
| if c != nil { | ||
| callbackDef = c.Callback() | ||
| } | ||
|
|
||
| if callbackDef == "" { | ||
| // if the callback is empty for the client, use the original behavior: | ||
| // /oidc/callback is simply appended to the original path, resulting in many different callback URLs | ||
| callbackDef = "oidc/callback" | ||
| } | ||
| return callbackDef | ||
| } | ||
|
|
||
| // CallbackURLForTarget returns the absolute URL to which IdP should redirect after authenticating the user. | ||
| // This is the speckial URL which is expected to be handled by the adapter under the provided Client configuration. | ||
| func CallbackURLForTarget(c Client, requestScheme, requestHost, requestPath string) string { | ||
| callbackDef := callbackDefinitionForClient(c) | ||
|
|
||
| if requestPath == "" { | ||
| requestPath = "/" | ||
| } | ||
|
|
||
| if callbackDef[0] == '/' { | ||
| // callback path absolute on the request host | ||
| return requestScheme + "://" + requestHost + callbackDef | ||
| } | ||
|
|
||
| m := regexpURL.FindStringSubmatch(callbackDef) | ||
| if len(m) > 1 { | ||
| // callback is full URL | ||
| return callbackDef | ||
| } | ||
|
|
||
| // callback path relative to the target path | ||
| if strings.HasSuffix(requestPath, callbackDef) { | ||
| // relative callback path already appended, do not append twice | ||
| // this can happen if this function is called in the actual callback handler | ||
| // (happens after /authorize OIDC provider URL redirects back and we call OIDC provider API again) | ||
| callbackDef = "" | ||
| } | ||
| return requestScheme + "://" + requestHost + path.Join(requestPath, callbackDef) | ||
| } | ||
|
|
||
| // IsCallbackRequest returns true if the provided request should be handled by the adapter as part of the auth flow | ||
| func IsCallbackRequest(c Client, requestScheme, requestHost, requestPath string) bool { | ||
| callbackDef := callbackDefinitionForClient(c) | ||
|
|
||
| if requestPath == "" { | ||
| requestPath = "/" | ||
| } | ||
|
|
||
| if callbackDef[0] == '/' { | ||
| // callback path absolute on the request host | ||
| return strings.HasPrefix(requestPath, callbackDef) | ||
| } | ||
|
|
||
| m := regexpURL.FindStringSubmatch(callbackDef) | ||
| if len(m) == 4 { | ||
| // callback is full URL, compare parts (case-insensitive just in case) | ||
| return strings.EqualFold(m[1], requestScheme) && | ||
| strings.EqualFold(m[2], requestHost) && | ||
| strings.EqualFold(m[3], requestPath) | ||
| } | ||
|
|
||
| // callback path relative to the target path, thus ending with callbackDef | ||
| return strings.HasSuffix(requestPath, callbackDef) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "net/url" | ||
| "testing" | ||
|
|
||
| "github.com/ibm-cloud-security/app-identity-and-access-adapter/tests/fake" | ||
| ) | ||
|
|
||
| func TestCallbackURLForTarget(t *testing.T) { | ||
| tests := []struct { | ||
| CallbackDef string | ||
| Scheme, Host, Path string | ||
| Result string | ||
| }{ | ||
| { // http scheme test | ||
| "", | ||
| "http", "test.com", "/admin", | ||
| "http://test.com/admin/oidc/callback", | ||
| }, | ||
| { // default relative callback | ||
| "", | ||
| "https", "test.com", "/admin", | ||
| "https://test.com/admin/oidc/callback", | ||
| }, | ||
| { // relative URI | ||
| "custom/relative/path", | ||
| "https", "test.com", "/admin", | ||
| "https://test.com/admin/custom/relative/path", | ||
| }, | ||
| { // relative URI already appended | ||
| "custom/relative/path", | ||
| "https", "test.com", "/admin/custom/relative/path", | ||
| "https://test.com/admin/custom/relative/path", | ||
| }, | ||
| { // absolute URI | ||
| "/absolute/uri/callback", | ||
| "https", "test.com", "/admin", | ||
| "https://test.com/absolute/uri/callback", | ||
| }, | ||
| { // full URL (http) | ||
| "http://test.com/my/callback", | ||
| "http", "test.com", "/admin", | ||
| "http://test.com/my/callback", | ||
| }, | ||
| { // full URL (https) | ||
| "https://test.com/my/callback", | ||
| "https", "test.com", "/admin", | ||
| "https://test.com/my/callback", | ||
| }, | ||
| } | ||
|
|
||
| for n, tst := range tests { | ||
| cli := fake.NewClientWithCallback(nil, tst.CallbackDef) | ||
|
|
||
| res := CallbackURLForTarget(cli, tst.Scheme, tst.Host, tst.Path) | ||
| if res != tst.Result { | ||
| t.Fatalf("TestCallbackURLForTarget#%d CallbackURLForTarget failed.\n"+ | ||
| " Expected: %s\n Got: %s", n, tst.Result, res) | ||
| } | ||
|
|
||
| resURL, err := url.Parse(res) | ||
| if err != nil { | ||
| t.Fatalf("TestCallbackURLForTarget#%d failed to parse URL returned from CallbackURLForTarget %s: %s", | ||
| n, res, err.Error()) | ||
| } | ||
|
|
||
| if !IsCallbackRequest(cli, resURL.Scheme, resURL.Host, resURL.Path) { | ||
| t.Fatalf("TestCallbackURLForTarget#%d IsCallbackRequest check not returning true\n"+ | ||
| " scheme: %s, host: %s, path: %s", n, resURL.Scheme, resURL.Host, resURL.Path) | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.