-
Notifications
You must be signed in to change notification settings - Fork 205
Initial draft PR to introduce Integration Tests for SAML auth flow #1044
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
Changes from all commits
04392e6
67adc8e
6c317b3
f8b69b1
9af841e
c56f6d1
3432bc4
2e181cf
4f09fac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC+eflDQihJ4zoZ | ||
| urwXuY3Nk9jRA3ms5Ge6jYuvId9mPjJ9ezndOdiVWnTgdqfmYUFMRsQ53iJJmprp | ||
| GHctBIPWpaz5Ao4oeco0D87CcySLA94tp8SG5ja2vasVYiNsaN9er1oWVo8uQP82 | ||
| iLGF5jXOBDaN3iPub8W2w78oSfgV2Kw5fgmcEIxUGyf/Jr1a8X5TdbX1rS8cZ08D | ||
| crZ8DAQZ9ndLl6wJ6RKnp7dqtEys1BT2LXQVvGkFp7zrN74AnNl8+FUE269Wct3t | ||
| sPVkIKgGxURGj+ky6oHT9MbVp7vvE9+7dcwfQMDqbd5elOIFZsWad0wd4LgIFUGa | ||
| ATnaSyXrAgMBAAECggEAGiwOWPSFLqnfONvUrnnbhyzSdN3CYUQ9EIAUemrwFE9l | ||
| hqJg8AnbvaHhP1pudZRVcZKjscPY+D4bHP40meXt65d2Lfzw5ZTeeMrXQRarJcLR | ||
| S3qq4VJOoEttb5G9hk7eqlbzzO/12ogpySd9JQXdzMH9cP7d9ww6oYNCB9oqEW4o | ||
| QVjwF5RHuq9dB6FmhsNux9VbbP6gGVTXA7IpmNwFddW8JoDM2mj952POYjAwEy00 | ||
| VQb0dzVrfuEw8DmpWZAOfyDt5QYO+7SC+L4eHbhrZrxeCzXr2hEhiCa1Nn3ieAHD | ||
| fF5FaguRMJcfGI3UEQZAw0++0SYeWPnjnwM1wE/nyQKBgQDyZA+6D1f3H/0fg4tT | ||
| ZrpGydnRPqqvMF+GBCtfa7Gjf6NdFQ+j0NzUXGzbIGy+0pJKEfEk4hE/2xFjCzru | ||
| LYJ5hSz/GXZH6qn4H39f0byl9oT6DYKDmrcXt1jRM4ioCU4Q1c58e2MqsvttVCnw | ||
| dVDQuTMb1xm6okHqcm3Fn8rUBwKBgQDJK7wbgrQcWriFxKDITdLvAivDcYAQ/p/e | ||
| XDw+nVdwq0p9Kpe+N5Lt6kvlTVyrS1aWfa9ucLVKMSKCbkjJBhesJsZU3nZgLa65 | ||
| XT5+sP4640/gBoHWvauYBBjOy0iHHzhrIx+Kw0BXWzrYE0ol0aIWYZt1/I1LQtoB | ||
| E0+ojQLN/QKBgQDCtZhgiNTLwhmOSBgSffHizWC4klN/+SayvASvWQ5QXUa4jiOL | ||
| H0tVF42mFIzmWLaE45bHXwYmOm7kFfBXxZ0Kyu0TWrvGF35Dv+GM8ilNVBML3vBZ | ||
| kV3EolapbnE3MopQQb/mBSPq9+26rCIoc8TgdfTVR1v2rUKv9w2w86R13wKBgGh5 | ||
| GQikeUscZiW6NtGvcPMFCptGb37j7Tx6ZCMUbVuq6VVVcFat39VEz0N3SMAAsSgY | ||
| f6n4SH4ORGC+S3hyfIq/3FIo8gsCznGflhwPaQhGEq5CUt2lxN5+ii+i7LiXoyIo | ||
| rHHQ8rIrQ8UBR4mac/XxnN3KWcqTHkpesAjVqnY1AoGAXoc9kr4v3WP0AG6lUhjB | ||
| P77AIrpCokbvnXfsAj9coPBJjK16XY534jjmnem4lwvxad/+PDWfU2i3wlP64YCi | ||
| BJkN8s9vJyJqHDFJjQ18zl6rdlFt+43x+YCt/zHsqazdcm47XN9kbXrunDYpVh2E | ||
| PZWa3aHt74aNhNMshynDIXI= | ||
| -----END PRIVATE KEY----- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIDVjCCAj4CCQCgL9L+gaw8+jANBgkqhkiG9w0BAQsFADBtMQswCQYDVQQGEwJV | ||
| UzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzEQ | ||
| MA4GA1UECgwHSmFua3lDbzEfMB0GA1UEAwwWVGVzdCBJZGVudGl0eSBQcm92aWRl | ||
| cjAeFw0yMjA3MTUxNjU1MjRaFw00MjA3MTAxNjU1MjRaMG0xCzAJBgNVBAYTAlVT | ||
| MRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQHDA1TYW4gRnJhbmNpc2NvMRAw | ||
| DgYDVQQKDAdKYW5reUNvMR8wHQYDVQQDDBZUZXN0IElkZW50aXR5IFByb3ZpZGVy | ||
| MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvnn5Q0IoSeM6Gbq8F7mN | ||
| zZPY0QN5rORnuo2LryHfZj4yfXs53TnYlVp04Han5mFBTEbEOd4iSZqa6Rh3LQSD | ||
| 1qWs+QKOKHnKNA/OwnMkiwPeLafEhuY2tr2rFWIjbGjfXq9aFlaPLkD/NoixheY1 | ||
| zgQ2jd4j7m/FtsO/KEn4FdisOX4JnBCMVBsn/ya9WvF+U3W19a0vHGdPA3K2fAwE | ||
| GfZ3S5esCekSp6e3arRMrNQU9i10FbxpBae86ze+AJzZfPhVBNuvVnLd7bD1ZCCo | ||
| BsVERo/pMuqB0/TG1ae77xPfu3XMH0DA6m3eXpTiBWbFmndMHeC4CBVBmgE52ksl | ||
| 6wIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQADzGujWOBelV81BoxkS5vB+VGmEDAx | ||
| I67cZCZ7734vJsrf5c6QV45zz+NiOqyLkY2JEsHMh89Ns8n+1MsbIPn01tfjXFgL | ||
| arJLhLBRxFhqZr0H81E8DAzHwjAtx8Qmr/IQXcLPhJ0SMubIGC7EhCkYrphteTyd | ||
| 2Rr5C9lCwF4Lb3xgoT2RsEO/IWDKb/CthcisQdDTw1XWLeAc+pJa76kOgDSkP93i | ||
| hHoZJMswOFU8KnLiXMaSxUZOXHLOYY7k4+xyh7dGqEkwKRYyY3TJ3mAULcJr5Ngz | ||
| UJvwmjmuEVCIgVNWqW45UsXJqkvdGFtUKj3UGfgyuvSV33daqXjkAims | ||
| -----END CERTIFICATE----- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| "lint:es": "node ../../scripts/eslint", | ||
| "lint:style": "node ../../scripts/stylelint", | ||
| "lint": "yarn run lint:es && yarn run lint:style", | ||
| "pretest:jest_server": "node ./test/jest_integration/runIdpServer.js &", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be started during
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this can be done, will only start idp server for the saml tests |
||
| "test:jest_server": "node ./test/run_jest_tests.js --config ./test/jest.config.server.js", | ||
| "test:jest_ui": "node ./test/run_jest_tests.js --config ./test/jest.config.ui.js" | ||
| }, | ||
|
|
@@ -25,7 +26,9 @@ | |
| "typescript": "4.0.2", | ||
| "gulp-rename": "2.0.0", | ||
| "@testing-library/react-hooks": "^7.0.2", | ||
| "@types/hapi__wreck": "^15.0.1" | ||
| "@types/hapi__wreck": "^15.0.1", | ||
| "selenium-webdriver": "^4.0.0-alpha.7", | ||
| "saml-idp": "^1.2.1" | ||
| }, | ||
| "dependencies": { | ||
| "@hapi/wreck": "^17.1.0", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ export class SamlAuthRoutes { | |
| validate: validateNextUrl, | ||
| }) | ||
| ), | ||
| redirectHash: schema.string(), | ||
| }), | ||
| }, | ||
| options: { | ||
|
|
@@ -64,6 +65,7 @@ export class SamlAuthRoutes { | |
| saml: { | ||
| nextUrl: request.query.nextUrl, | ||
| requestId: samlHeader.requestId, | ||
| redirectHash: request.query.redirectHash === 'true', | ||
| }, | ||
| }; | ||
| this.sessionStorageFactory.asScoped(request).set(cookie); | ||
|
|
@@ -81,7 +83,8 @@ export class SamlAuthRoutes { | |
|
|
||
| this.router.post( | ||
| { | ||
| path: '/_plugins/_security/saml/acs', | ||
| // need to change to /_opendistro to execute the tests. | ||
| path: '/_opendistro/_security/saml/acs', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way you can test both endpoints or just the new one?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can ignore this, since you had already merged this change |
||
| validate: { | ||
| body: schema.any(), | ||
| }, | ||
|
|
@@ -92,13 +95,15 @@ export class SamlAuthRoutes { | |
| async (context, request, response) => { | ||
| let requestId: string = ''; | ||
| let nextUrl: string = '/'; | ||
| let redirectHash: boolean = false; | ||
| try { | ||
| const cookie = await this.sessionStorageFactory.asScoped(request).get(); | ||
| if (cookie) { | ||
| requestId = cookie.saml?.requestId || ''; | ||
| nextUrl = | ||
| cookie.saml?.nextUrl || | ||
| `${this.coreSetup.http.basePath.serverBasePath}/app/opensearch-dashboards`; | ||
| redirectHash = cookie.saml?.redirectHash || false; | ||
| } | ||
| if (!requestId) { | ||
| return response.badRequest({ | ||
|
|
@@ -140,11 +145,22 @@ export class SamlAuthRoutes { | |
| expiryTime, | ||
| }; | ||
| this.sessionStorageFactory.asScoped(request).set(cookie); | ||
| return response.redirected({ | ||
| headers: { | ||
| location: nextUrl, | ||
| }, | ||
| }); | ||
| if (redirectHash) { | ||
| console.log('The server base path is : ' + this.coreSetup.http.basePath.serverBasePath); | ||
| return response.redirected({ | ||
| headers: { | ||
| location: `${ | ||
| this.coreSetup.http.basePath.serverBasePath | ||
| }/auth/saml/redirectUrlFragment?nextUrl=${escape(nextUrl)}`, | ||
| }, | ||
| }); | ||
| } else { | ||
| return response.redirected({ | ||
| headers: { | ||
| location: nextUrl, | ||
| }, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| context.security_plugin.logger.error( | ||
| `SAML SP initiated authentication workflow failed: ${error}` | ||
|
|
@@ -212,6 +228,111 @@ export class SamlAuthRoutes { | |
| } | ||
| ); | ||
|
|
||
| this.coreSetup.http.resources.register( | ||
| { | ||
| path: '/auth/saml/captureUrlFragment', | ||
| validate: { | ||
| query: schema.object({ | ||
| nextUrl: schema.maybe( | ||
| schema.string({ | ||
| validate: validateNextUrl, | ||
| }) | ||
| ), | ||
| }), | ||
| }, | ||
| options: { | ||
| authRequired: false, | ||
| }, | ||
| }, | ||
| async (context, request, response) => { | ||
| this.sessionStorageFactory.asScoped(request).clear(); | ||
| const serverBasePath = this.coreSetup.http.basePath.serverBasePath; | ||
| return response.renderHtml({ | ||
| body: ` | ||
| <!DOCTYPE html> | ||
| <title>OSD SAML Capture</title> | ||
| <link rel="icon" href="data:,"> | ||
| <script src="${serverBasePath}/auth/saml/captureUrlFragment.js"></script> | ||
| `, | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| this.coreSetup.http.resources.register( | ||
| { | ||
| path: '/auth/saml/captureUrlFragment.js', | ||
| validate: false, | ||
| options: { | ||
| authRequired: false, | ||
| }, | ||
| }, | ||
| async (context, request, response) => { | ||
| this.sessionStorageFactory.asScoped(request).clear(); | ||
| return response.renderJs({ | ||
| body: `let samlHash=window.location.hash.toString(); | ||
| let redirectHash = false; | ||
| if (samlHash !== "") { | ||
| window.localStorage.removeItem('samlHash'); | ||
| window.localStorage.setItem('samlHash', samlHash); | ||
| redirectHash = true; | ||
| } | ||
| let params = new URLSearchParams(window.location.search); | ||
| let nextUrl = params.get("nextUrl"); | ||
| finalUrl = "login?nextUrl=" + encodeURIComponent(nextUrl); | ||
| finalUrl += "&redirectHash=" + encodeURIComponent(redirectHash); | ||
| window.location.replace(finalUrl); | ||
|
|
||
| `, | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| this.coreSetup.http.resources.register( | ||
| { | ||
| path: '/auth/saml/redirectUrlFragment', | ||
| validate: { | ||
| query: schema.object({ | ||
| nextUrl: schema.any(), | ||
| }), | ||
| }, | ||
| options: { | ||
| authRequired: true, | ||
| }, | ||
| }, | ||
| async (context, request, response) => { | ||
| const serverBasePath = this.coreSetup.http.basePath.serverBasePath; | ||
| return response.renderHtml({ | ||
| body: ` | ||
| <!DOCTYPE html> | ||
| <title>OSD SAML Success</title> | ||
| <link rel="icon" href="data:,"> | ||
| <script src="${serverBasePath}/auth/saml/redirectUrlFragment.js"></script> | ||
| `, | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| this.coreSetup.http.resources.register( | ||
| { | ||
| path: '/auth/saml/redirectUrlFragment.js', | ||
| validate: false, | ||
| options: { | ||
| authRequired: true, | ||
| }, | ||
| }, | ||
| async (context, request, response) => { | ||
| return response.renderJs({ | ||
| body: `let samlHash=window.localStorage.getItem('samlHash'); | ||
| window.localStorage.removeItem('samlHash'); | ||
| let params = new URLSearchParams(window.location.search); | ||
| let nextUrl = params.get("nextUrl"); | ||
| finalUrl = nextUrl + samlHash; | ||
| window.location.replace(finalUrl); | ||
| `, | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| this.router.get( | ||
| { | ||
| path: API_AUTH_LOGOUT, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| const { runServer } = require('saml-idp'); | ||
|
|
||
| // Create certificate pair on the fly and pass it to runServer | ||
| runServer({ | ||
| acsUrl: 'http://localhost:5601/_opendistro/_security/saml/acs', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible to run idp server with acs
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did not understand, can you please explain? |
||
| audience: 'https://localhost:9200', | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a reference, can we remove both checked in certs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the IDP server uses this certificate to generate idp's metadata, we can acutally generate these certificates on the fly, I have just hardcoded for now