diff --git a/e2e/integration/spec/gateway/v2_app_policies.spec.js b/e2e/integration/spec/gateway/v2_app_policies.spec.js index 5962a34ec..44d461734 100644 --- a/e2e/integration/spec/gateway/v2_app_policies.spec.js +++ b/e2e/integration/spec/gateway/v2_app_policies.spec.js @@ -1,5 +1,6 @@ const client = require('../../utils/client'); const { delay } = require('../../utils/utils'); +const jsonpatch = require('fast-json-patch'); const policies = [ { @@ -12,7 +13,7 @@ const policies = [ }, ]; -describe('Gateway v2 - App Policies', () => { +describe.only('Gateway v2 - App Policies', () => { const cases = [ { name: 'read_specific_key', @@ -108,6 +109,23 @@ describe('Gateway v2 - App Policies', () => { }, ]; + let originalPolicies; + let currentPolicies; + + before(async () => { + const policiesRes = await client.get('/api/v2/policies').expect(200); + originalPolicies = policiesRes.body; + currentPolicies = originalPolicies; + }); + + after(async () => { + const policiesRes = await client.get('/api/v2/policies').expect(200); + newCurrentPolicies = policiesRes.body; + + const patch = jsonpatch.compare(newCurrentPolicies, originalPolicies); + await client.patch('/api/v2/policies').send(patch).expect(200); + }); + for (let policy of policies) { it(`testing permission ${policy.object} - ${policy.action}`, async () => { const relevantCases = cases.filter( @@ -127,23 +145,24 @@ describe('Gateway v2 - App Policies', () => { .expect(200); const { appId, appSecret } = response.body; - - await client - .patch('/api/v2/policies') - .send([ + const newPolicies = { + policies: [ + ...currentPolicies.policies, { - op: 'add', - path: '/policies/1', - value: { - ...policy, - group: 'externalapps', - user: appId, - contexts: {}, - effect: 'allow', - }, + group: '*', + user: '*', + object: 'repo', + action: 'read', + group: 'externalapps', + user: appId, + contexts: {}, + effect: 'allow', }, - ]) - .expect(200); + ], + }; + const patch = jsonpatch.compare(currentPolicies, newPolicies); + await client.patch('/api/v2/policies').send(patch).expect(200); + currentPolicies = newPolicies; const appClient = await client.with((client) => client.set({ 'x-client-id': appId, 'x-client-secret': appSecret }).unset('Authorization'), diff --git a/e2e/integration/spec/gateway/v2_hooks_policies.spec.js b/e2e/integration/spec/gateway/v2_hooks_policies.spec.js new file mode 100644 index 000000000..52e57f678 --- /dev/null +++ b/e2e/integration/spec/gateway/v2_hooks_policies.spec.js @@ -0,0 +1,98 @@ +const client = require('../../utils/client'); +const { delay } = require('../../utils/utils'); +const jsonpatch = require('fast-json-patch'); + +describe(`testing to make sure hooks is only readable if authenticated `, () => { + const authorizedAppId = 'auth-app-id'; + const unAuthorizedAppId = 'unauth-app-id'; + + it('authorized app gets 200', async () => { + const response = await client + .post('/api/v2/apps') + .send({ name: authorizedAppId, permissions: [] }) + .expect(200); + + const { appId, appSecret } = response.body; + + const policiesRes = await client.get('/api/v2/policies').expect(200); + const currentPolicies = policiesRes.body; + const newPolicies = { + policies: [ + ...currentPolicies.policies, + { + group: 'externalapps', + user: appId, + object: 'repo', + contexts: {}, + action: 'read', + effect: 'allow', + }, + { + group: 'externalapps', + user: appId, + object: 'repo/hooks', + contexts: {}, + action: 'read', + effect: 'allow', + } + ], + }; + const patch = jsonpatch.compare(currentPolicies, newPolicies); + + await client + .patch('/api/v2/policies') + .send(patch) + .expect(200); + + const appClient = await client.with((client) => + client + .set({ 'x-client-id': appId, 'x-client-secret': appSecret }) + .unset('Authorization'), + ); + + await delay(3000); + await appClient.get('/api/v2/schemas').expect(200); + await appClient.get('/api/v2/hooks').expect(200); + }); + + it('unauthorized app gets 403', async () => { + const response = await client + .post('/api/v2/apps') + .send({ name: unAuthorizedAppId, permissions: [] }) + .expect(200); + + const { appId, appSecret } = response.body; + + const policiesRes = await client.get('/api/v2/policies').expect(200); + const currentPolicies = policiesRes.body; + const newPolicies = { + policies: [ + ...currentPolicies.policies, + { + group: 'externalapps', + user: appId, + object: 'repo', + contexts: {}, + action: 'read', + effect: 'allow', + } + ], + }; + const patch = jsonpatch.compare(currentPolicies, newPolicies); + + await client + .patch('/api/v2/policies') + .send(patch) + .expect(200); + + const appClient = await client.with((client) => + client + .set({ 'x-client-id': appId, 'x-client-secret': appSecret }) + .unset('Authorization'), + ); + + await delay(3000); + await appClient.get('/api/v2/schemas').expect(200); + await appClient.get('/api/v2/hooks').expect(403); + }); +}); \ No newline at end of file diff --git a/services/gateway/go.mod b/services/gateway/go.mod index 1c8d73f96..3354a9404 100644 --- a/services/gateway/go.mod +++ b/services/gateway/go.mod @@ -19,7 +19,7 @@ require ( github.com/open-policy-agent/opa v0.28.0 github.com/prometheus/client_golang v1.10.0 github.com/rs/cors v1.7.0 - github.com/sirupsen/logrus v1.8.1 + github.com/sirupsen/logrus v1.9.0 github.com/urfave/negroni v1.0.0 github.com/vulcand/oxy v1.3.0 golang.org/x/crypto v0.0.0-20201217014255-9d1352758620 diff --git a/services/gateway/go.sum b/services/gateway/go.sum index 434088a89..d044c61f6 100644 --- a/services/gateway/go.sum +++ b/services/gateway/go.sum @@ -338,6 +338,8 @@ github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6Mwd github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE= github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= +github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= +github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/goconvey v1.6.4 h1:fv0U8FUIMPNf1L9lnHLvLhgicrIVChEkdzIKYqbNC9s= @@ -468,6 +470,8 @@ golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210309074719-68d13333faf2 h1:46ULzRKLh1CwgRq2dC5SlBzEqqNCi8rreOZnNrbqcIY= golang.org/x/sys v0.0.0-20210309074719-68d13333faf2/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/services/gateway/security/requestUtils.go b/services/gateway/security/requestUtils.go index d4bcadade..f915e7e8e 100644 --- a/services/gateway/security/requestUtils.go +++ b/services/gateway/security/requestUtils.go @@ -147,8 +147,11 @@ func extractResourceFromRepoRequest(r *http.Request, u UserInfo, kind string) (c ctxs = PolicyResource{Contexts: map[string]string{}} switch { case r.Method == "GET": - ctxs.Item = "repo" - break + if kind != "hooks" { + ctxs.Item = "repo" + break + } + fallthrough case r.Method == "POST": fallthrough case r.Method == "PUT": @@ -157,10 +160,10 @@ func extractResourceFromRepoRequest(r *http.Request, u UserInfo, kind string) (c fallthrough case r.Method == "DELETE": ctxs.Item = "repo/" + kind - break default: err = fmt.Errorf("Invalid method %s for %s", r.Method, kind) } + return } diff --git a/services/gateway/security/requestUtils_test.go b/services/gateway/security/requestUtils_test.go index 1c685721d..f5cd2f9a0 100644 --- a/services/gateway/security/requestUtils_test.go +++ b/services/gateway/security/requestUtils_test.go @@ -222,7 +222,7 @@ func TestExtractFromRequest(t *testing.T) { args: args{ r: createTestRequest("GET", "https://gateway.tweek.com/api/v2/hooks", userInfo), }, - wantObj: PolicyResource{Item: "repo", Contexts: map[string]string{}}, + wantObj: PolicyResource{Item: "repo/hooks", Contexts: map[string]string{}}, wantSub: &Subject{User: "A b sub", Group: "default"}, wantAct: "read", wantErr: nil, diff --git a/services/gateway/version.go b/services/gateway/version.go index 527314570..a417aab81 100644 --- a/services/gateway/version.go +++ b/services/gateway/version.go @@ -1,3 +1,3 @@ package main -const Version = "1.0.0-rc20" +const Version = "1.0.0-rc21" diff --git a/services/git-service/BareRepository/tests-source/security/policy.json b/services/git-service/BareRepository/tests-source/security/policy.json index 7dd10354e..8f4ac43ed 100644 --- a/services/git-service/BareRepository/tests-source/security/policy.json +++ b/services/git-service/BareRepository/tests-source/security/policy.json @@ -113,6 +113,14 @@ "object": "*", "action": "*", "effect": "allow" + }, + { + "group": "*", + "user": "*", + "contexts": {}, + "object": "repo/hooks", + "action": "read", + "effect": "allow" } ] }