From b0f9f529e96dc479c05932d7cad26ac3284da88d Mon Sep 17 00:00:00 2001 From: Thorsten Lanfer Date: Wed, 28 Aug 2019 14:49:29 +0200 Subject: [PATCH 1/5] add support for idp app sessions --- idp/authmiddleware.go | 8 ++++++++ idp/authmiddleware_test.go | 38 +++++++++++++++++++++++++++++++++++++- idp/scim/user.go | 10 ++++++++++ idp/scim/user_test.go | 13 +++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/idp/authmiddleware.go b/idp/authmiddleware.go index fea6e3e..b3db04b 100644 --- a/idp/authmiddleware.go +++ b/idp/authmiddleware.go @@ -295,3 +295,11 @@ func AuthSessionIdFromCtx(ctx context.Context) (string, error) { } return authSessionId, nil } + +func AppFromCtx(ctx context.Context) (string, bool) { + if principal, err := PrincipalFromCtx(ctx); err == nil { + return principal.App() + } else { + return "", false + } +} diff --git a/idp/authmiddleware_test.go b/idp/authmiddleware_test.go index 87b0d51..bf7f820 100644 --- a/idp/authmiddleware_test.go +++ b/idp/authmiddleware_test.go @@ -599,7 +599,7 @@ func TestRequestAsExternalUserAndExternalValidationIsAllowed_PopulatesContextWit } } -func TestRequestAsInternalUserAndExternalValidationIsAllowed_PopulatesContextWithPrincipalAndAuthsession(t *testing.T) { +func TestRequestAsInternalUserAndExternalValidationIsAllowed_PopulatesContextWithAppPrincipalAndAuthsession(t *testing.T) { req, err := http.NewRequest("GET", "/myresource/subresource?query1=abc&query2=123", nil) if err != nil { t.Fatal(err) @@ -621,6 +621,29 @@ func TestRequestAsInternalUserAndExternalValidationIsAllowed_PopulatesContextWit } } +func TestRequestAsApp_PopulatesContextWithPrincipalAndAuthsession(t *testing.T) { + req, err := http.NewRequest("GET", "/myresource/subresource?query1=abc&query2=123", nil) + if err != nil { + t.Fatal(err) + } + const authSessionId = "2XGxJeb0q+/fS8biFi8FE7TovJPPEPyzlDxT6bh5p5pHA/x7CEi1w9egVhEMz8IWasdfJRFnkSqJnLr61cOKf/i5eWuu7Duh+OTtTjMOt9w=&Bnh4NNU90wH_OVlgbzbdZOEu1aSuPlbUctiCdYTonZ3Ap_Zd3bVL79I-dPdHf4OOgO8NKEdqyLsqc8RhAOreXgJqXuqsreeI" + principal := scim.Principal{Id: "some-app@app.idp.d-velop.local"} + req.Header.Set("Authorization", "Bearer "+authSessionId) + handlerSpy := new(handlerSpy) + idpStub := newIdpStub(map[string]scim.Principal{authSessionId: principal}, nil) + defer idpStub.Close() + + idp.HandleAuth(returnFromCtx(idpStub.URL), returnFromCtx("1"), true, log, log)(handlerSpy).ServeHTTP(httptest.NewRecorder(), req) + + if err := handlerSpy.assertAuthSessionIdIs(authSessionId); err != nil { + t.Error(err) + } + + if err := handlerSpy.assertPrincipalIsApp("some-app"); err != nil { + t.Error(err) + } +} + func TestIdpSendsNoCacheHeader_CallsIdp(t *testing.T) { req, err := http.NewRequest("GET", "/myresource/subresource?query1=abc&query2=123", nil) if err != nil { @@ -752,12 +775,15 @@ type handlerSpy struct { authSessionId string prinicpal scim.Principal hasBeenCalled bool + app string + isApp bool } func (spy *handlerSpy) ServeHTTP(rw http.ResponseWriter, r *http.Request) { spy.hasBeenCalled = true spy.authSessionId, _ = idp.AuthSessionIdFromCtx(r.Context()) spy.prinicpal, _ = idp.PrincipalFromCtx(r.Context()) + spy.app, spy.isApp = idp.AppFromCtx(r.Context()) } func (spy *handlerSpy) assertAuthSessionIdIs(expectedAuthSessionID string) error { @@ -774,6 +800,16 @@ func (spy *handlerSpy) assertPrincipalIs(expectedPrincipal scim.Principal) error return nil } +func (spy *handlerSpy) assertPrincipalIsApp(expectedApp string) error { + if !spy.isApp { + return fmt.Errorf("handler set non-app principal, got %+v", spy.prinicpal) + } + if spy.app != expectedApp { + return fmt.Errorf("handler set wrong app, got %v, want %v", spy.app, expectedApp) + } + return nil +} + type responseSpy struct { *httptest.ResponseRecorder } diff --git a/idp/scim/user.go b/idp/scim/user.go index 4723d89..7e2fdb3 100644 --- a/idp/scim/user.go +++ b/idp/scim/user.go @@ -6,8 +6,11 @@ package scim import ( "encoding/json" + "strings" ) +const appSessionIdSuffix = "@app.idp.d-velop.local" + // Principal represents a user. // // It complies to the SCIM User Schema. @@ -70,6 +73,13 @@ func (p Principal) String() string { return string(b) } +func (p Principal) App() (string, bool) { + if strings.HasSuffix(p.Id, appSessionIdSuffix) { + return strings.TrimSuffix(p.Id, appSessionIdSuffix), true + } + return "", false +} + type UserName struct { // Formatted is the full name, including all middle names, titles, and suffixes as appropriate, formatted for display (e.g. Ms. Barbara Jane Jensen, III.). Formatted string `json:"formatted"` diff --git a/idp/scim/user_test.go b/idp/scim/user_test.go index 0719a97..2e66f2d 100644 --- a/idp/scim/user_test.go +++ b/idp/scim/user_test.go @@ -22,3 +22,16 @@ func TestCanDeserializeSCIMUser(t *testing.T) { t.Errorf("Unmarshaled Object wrong: got \n %v want\n %v", u, donaldDuck) } } + +func TestUserHasAppId_ReturnsApp( t *testing.T) { + principal := scim.Principal{Id: "some-app@app.idp.d-velop.local"} + app, isApp := principal.App() + + if !isApp { + t.Errorf("expected isApp = true, got false") + } + + if app != "some-app" { + t.Errorf("expected app = 'some-app', got %v", app) + } +} \ No newline at end of file From 25a745cd548208ef6de8c2598cd019bdd300989d Mon Sep 17 00:00:00 2001 From: Thorsten Lanfer Date: Wed, 28 Aug 2019 14:56:45 +0200 Subject: [PATCH 2/5] add testcase for requesting app on a non-app session --- idp/authmiddleware_test.go | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/idp/authmiddleware_test.go b/idp/authmiddleware_test.go index bf7f820..9f892b8 100644 --- a/idp/authmiddleware_test.go +++ b/idp/authmiddleware_test.go @@ -599,7 +599,7 @@ func TestRequestAsExternalUserAndExternalValidationIsAllowed_PopulatesContextWit } } -func TestRequestAsInternalUserAndExternalValidationIsAllowed_PopulatesContextWithAppPrincipalAndAuthsession(t *testing.T) { +func TestRequestAsInternalUserAndExternalValidationIsAllowed_PopulatesContextWithPrincipalAndAuthsession(t *testing.T) { req, err := http.NewRequest("GET", "/myresource/subresource?query1=abc&query2=123", nil) if err != nil { t.Fatal(err) @@ -639,7 +639,30 @@ func TestRequestAsApp_PopulatesContextWithPrincipalAndAuthsession(t *testing.T) t.Error(err) } - if err := handlerSpy.assertPrincipalIsApp("some-app"); err != nil { + if err := handlerSpy.assertPrincipalIsApp(true, "some-app"); err != nil { + t.Error(err) + } +} + +func TestRequestAsNonApp_RequestAppPrincipal_ReturnsNoApp(t *testing.T) { + req, err := http.NewRequest("GET", "/myresource/subresource?query1=abc&query2=123", nil) + if err != nil { + t.Fatal(err) + } + const authSessionId = "2XGxJeb0q+/fS8biFi8FE7TovJPPEPyzlDxT6bh5p6pHA/x7CEi1w9egVhEMz8IWasdfJRFnkSqJnLr61cOKf/i5eWuu7Duh+OTtTjMOt9w=&Bnh4NNU90wH_OVlgbzbdZOEu1aSuPlbUctiCdYTonZ3Ap_Zd3bVL79I-dPdHf4OOgO8NKEdqyLsqc8RhAOreXgJqXuqsreeI" + principal := scim.Principal{Id: "7bbcf1b6-017a-449a-ad5f-9723d28223e1"} + req.Header.Set("Authorization", "Bearer "+authSessionId) + handlerSpy := new(handlerSpy) + idpStub := newIdpStub(map[string]scim.Principal{authSessionId: principal}, nil) + defer idpStub.Close() + + idp.HandleAuth(returnFromCtx(idpStub.URL), returnFromCtx("1"), true, log, log)(handlerSpy).ServeHTTP(httptest.NewRecorder(), req) + + if err := handlerSpy.assertAuthSessionIdIs(authSessionId); err != nil { + t.Error(err) + } + + if err := handlerSpy.assertPrincipalIsApp(false, ""); err != nil { t.Error(err) } } @@ -800,9 +823,9 @@ func (spy *handlerSpy) assertPrincipalIs(expectedPrincipal scim.Principal) error return nil } -func (spy *handlerSpy) assertPrincipalIsApp(expectedApp string) error { - if !spy.isApp { - return fmt.Errorf("handler set non-app principal, got %+v", spy.prinicpal) +func (spy *handlerSpy) assertPrincipalIsApp(expectIsApp bool, expectedApp string) error { + if spy.isApp != expectIsApp { + return fmt.Errorf("handler set isApp = '%v', want '%v'", spy.isApp, expectIsApp) } if spy.app != expectedApp { return fmt.Errorf("handler set wrong app, got %v, want %v", spy.app, expectedApp) From 01776de5a86a1aac878ffde0ef44fc6244ebadd2 Mon Sep 17 00:00:00 2001 From: Thorsten Lanfer Date: Thu, 29 Aug 2019 09:03:34 +0200 Subject: [PATCH 3/5] give names to return values for better code generation --- idp/authmiddleware.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/idp/authmiddleware.go b/idp/authmiddleware.go index b3db04b..378c806 100644 --- a/idp/authmiddleware.go +++ b/idp/authmiddleware.go @@ -296,7 +296,7 @@ func AuthSessionIdFromCtx(ctx context.Context) (string, error) { return authSessionId, nil } -func AppFromCtx(ctx context.Context) (string, bool) { +func AppFromCtx(ctx context.Context) (appName string, isApp bool) { if principal, err := PrincipalFromCtx(ctx); err == nil { return principal.App() } else { From 16ac4a2e5a44628d8610a299b043a818076a1981 Mon Sep 17 00:00:00 2001 From: Thorsten Lanfer Date: Thu, 29 Aug 2019 09:09:49 +0200 Subject: [PATCH 4/5] give names to return values for better code generation --- idp/scim/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/idp/scim/user.go b/idp/scim/user.go index 7e2fdb3..81f3ee1 100644 --- a/idp/scim/user.go +++ b/idp/scim/user.go @@ -73,7 +73,7 @@ func (p Principal) String() string { return string(b) } -func (p Principal) App() (string, bool) { +func (p Principal) App() (appName string, isApp bool) { if strings.HasSuffix(p.Id, appSessionIdSuffix) { return strings.TrimSuffix(p.Id, appSessionIdSuffix), true } From 99fe421eed28be0396c5a5cfa7decad36bf8300e Mon Sep 17 00:00:00 2001 From: Thorsten Lanfer Date: Thu, 29 Aug 2019 09:11:13 +0200 Subject: [PATCH 5/5] add testcase for getting app info on a non-app scim.Prinzipal --- idp/scim/user_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/idp/scim/user_test.go b/idp/scim/user_test.go index 2e66f2d..8e6973b 100644 --- a/idp/scim/user_test.go +++ b/idp/scim/user_test.go @@ -34,4 +34,16 @@ func TestUserHasAppId_ReturnsApp( t *testing.T) { if app != "some-app" { t.Errorf("expected app = 'some-app', got %v", app) } +} + +func TestUserHasNonAppId_ReturnsNoApp( t *testing.T) { + appName, isApp := donaldDuck.App() + + if isApp { + t.Errorf("expected isApp = false") + } + + if appName != "" { + t.Errorf("expected appName = '', got appNAme = '%v'", appName) + } } \ No newline at end of file