From 1b158be6ed990e9536af23b03d548cd82182c2e0 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Fri, 9 Aug 2024 11:56:34 -0700 Subject: [PATCH 1/8] Scaffold and add some CAPire links --- .../bad-authn-authz/EntityExposedWithoutAuthn.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md new file mode 100644 index 000000000..6c3bd6100 --- /dev/null +++ b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md @@ -0,0 +1,13 @@ +# CAP Entities Exposed without Access Controls + +## Recommendation + +## Examples + +## References + +- SAP CAPire Documentation: [Authorization Enforcement](https://cap.cloud.sap/docs/node.js/authentication#enforcement). +- SAP CAPire Documentation: [@restrict](https://cap.cloud.sap/docs/guides/security/authorization#restrict-annotation). +- SAP CAPire Documentation: +[@requires](https://cap.cloud.sap/docs/guides/security/authorization#requires). +- SAP CAPire Documentation: [Protecting Certain Entries](https://cap.cloud.sap/docs/cds/common#protecting-certain-entries). From 56d7af604fa5c057dcdfcb012e7835e5a35cff9a Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Fri, 9 Aug 2024 15:35:22 -0700 Subject: [PATCH 2/8] Add first draft of `EntityExposedWithoutAuthn.md` --- .../EntityExposedWithoutAuthn.md | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md index 6c3bd6100..1f7d2a4c6 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md @@ -2,8 +2,76 @@ ## Recommendation +### Use CDS-based authorization + +CDL provides two annotations to declare access controls `@requires` and `@restrict` with the latter providing more granularity than the former. For example, to check if a request is being made by an authenticated user to the CDL entity or service, annotate it with `@requires: 'authenticated-user'`. On the other hand, if it needs to be read only via a certain group of users where the user has level greater than 2, use `@restrict: { grant: 'READ', to: 'SomeUser', where: { $user.level > 2 } }` (note the leading `$`). + +#### Check the original CDS entity it is derived from + +CDS entities may be derived from other entities by means of selection and projection. These derived definitions inherit its identical conditions for access control or can choose to override them. Therefore, in order to accurately determine what authorization an entity requires, the access control of the parent entity should be transitively inspected (that is, the inspection should go up the chain of derivation). + +### Enforce authorization with JavaScript + +Access control may be enforced when a handler for requests to the relevant entity or service is registered to a service. Both `cds.Service.before` and `cds.Service.on` may be used to enforce it. For example, to restrict writing to and updating an entity to certain role satisfying some requirements, the below handler registrations may be used interchangably: + +``` javascript +/** + * Intercept requests to SomeEntity before access and check if the request + * is coming from a user with SomeRole and level greater than 3. + */ +this.before(["WRITE", "UPDATE"], "SomeEntity", (req) => { + (req.user.is("SomeRole") && req.user.attr.level > 3) || req.reject(403); +}); + +/** + * On request to access SomeEntity, check if the request is coming from a user + * with SomeRole and level greater than 3. + */ +this.on(["WRITE", "UPDATE"], "SomeEntity", (req) => { + if (req.user.is("SomeRole") && req.user.attr.level > 3) { + /* Do something */ + } else req.reject(403); +}); +``` + ## Examples +The following CDS definition and its JavaScript implementation imposes no authorization on SomeEntity. Note that the `OriginalEntity` from which `DerivedEntity` derives from does not control access to it, either. + +### db/schema.cds + +``` cap-cds +namespace sample_namespace.sample_entities; + +entity OriginalEntity { + Attribute1 : String(100); + Attribute2 : String(100) +} +``` + +### srv/service1.cds + +``` cap-cds +using { sample_namespace.sample_entities as db_schema } from '../db/schema'; + +service SomeService { + entity DerivedEntity as projection on db_schema.OriginalEntity excluding { Attribute2 } +} +``` + +### srv/service1.js + +``` javascript + +const cds = require("@sap/cds"); + +module.exports = class Service1 extends cds.ApplicationService { + init() { + this.on("READ", "SomeService", (req) => { }) + } +} +``` + ## References - SAP CAPire Documentation: [Authorization Enforcement](https://cap.cloud.sap/docs/node.js/authentication#enforcement). @@ -11,3 +79,7 @@ - SAP CAPire Documentation: [@requires](https://cap.cloud.sap/docs/guides/security/authorization#requires). - SAP CAPire Documentation: [Protecting Certain Entries](https://cap.cloud.sap/docs/cds/common#protecting-certain-entries). +- SAP CAPire Documentation: [Inheritance of Restrictions](https://cap.cloud.sap/docs/guides/security/authorization#inheritance-of-restrictions). +- Common Weakness Enumeration: [CWE-862](https://cwe.mitre.org/data/definitions/862.html). +- Commoon Weakness Enumeration: [CWE-306](https://cwe.mitre.org/data/definitions/306.html). + From c378a6775deb707959e5a2a3c7240d52100b9aa4 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Mon, 12 Aug 2024 11:33:06 -0700 Subject: [PATCH 3/8] Update --- .../DefaultUserIsPrivileged.md | 50 +++++++++++++++++++ .../EntityExposedWithoutAuthn.md | 10 ++-- 2 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md diff --git a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md new file mode 100644 index 000000000..c88cda2d3 --- /dev/null +++ b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md @@ -0,0 +1,50 @@ +# Default User is overwritten as privileged + +The user whose request cannot be verified as authenticated is represented as `cds.User.default` internally. If this property is set to `cds.User.Privileged`, then a service may provide assets to some user who has no rights to access them. + +## Recommendation + +### Set up a development profile that uses non-production authentication + +It may be tempting to overwrite the `cds.User.default` as `cds.User.Privileged`, for ease of development. However, this may slip through production undeleted since the assignment to `cds.User.default` can be hard to detect because it may take various forms; e.g. the programmer may choose to store `cds.User` to a variable `v` and access `cds.User.default` by `v.default`. + +A safer and more elegant solution is to set up a development profile and opt in to use a non-production strategy such as basic, dummy, or mocked during its use. This can be done in `package.json` of the CAP application at its project root: + +``` json +{ + "requires": { + "[dev]": { + "auth": "dummy" + } + } +} +``` + +Setting `"dummy"` as the development authentication strategy has the effect of disabling `@requires` and `@restrict` annotations of CDS definitions that provides authorization. The application during development then can be run and tested with the `--profile dev` option. + +```shell +cds serve --profile dev +``` + +## Example + +Setting `cds.User.default` to `cds.User.Privileged` may happen anywhere in the application. The following is the server.js that provides the top-level definition of a CAP application, overwriting the property with the problematic class. + +``` javascript +const cds = require("@sap/cds"); +const app = require("express")(); + +/* + * Antipattern: `cds.User.default` is overwritten to `cds.User.Privileged` + */ +cds.User.default = cdsUser.Privileged; + +cds.serve("all").in(app); +``` + +## References + +- SAP CAPire Documentation: [cds.User.default](https://cap.cloud.sap/docs/node.js/authentication#default-user). +- SAP CAPire Documentation: [Authentication Strategies](https://cap.cloud.sap/docs/node.js/authentication#strategies). +- Common Weakness Enumeration: [CWE-862](https://cwe.mitre.org/data/definitions/862.html). +- Common Weakness Enumeration: [CWE-306](https://cwe.mitre.org/data/definitions/306.html). diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md index 1f7d2a4c6..fda371bd2 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md @@ -1,11 +1,13 @@ -# CAP Entities Exposed without Access Controls +# CAP Definitions Exposed without Access Controls + +Although using a production-level authentication strategy such as `jwt` ensures that all entities and services require the user to be authenticated, this does not guarantee any further authorization. Furthermore, the lack of required authentication or authorization may imply a gap in the design of the system. ## Recommendation ### Use CDS-based authorization CDL provides two annotations to declare access controls `@requires` and `@restrict` with the latter providing more granularity than the former. For example, to check if a request is being made by an authenticated user to the CDL entity or service, annotate it with `@requires: 'authenticated-user'`. On the other hand, if it needs to be read only via a certain group of users where the user has level greater than 2, use `@restrict: { grant: 'READ', to: 'SomeUser', where: { $user.level > 2 } }` (note the leading `$`). - + #### Check the original CDS entity it is derived from CDS entities may be derived from other entities by means of selection and projection. These derived definitions inherit its identical conditions for access control or can choose to override them. Therefore, in order to accurately determine what authorization an entity requires, the access control of the parent entity should be transitively inspected (that is, the inspection should go up the chain of derivation). @@ -80,6 +82,6 @@ module.exports = class Service1 extends cds.ApplicationService { [@requires](https://cap.cloud.sap/docs/guides/security/authorization#requires). - SAP CAPire Documentation: [Protecting Certain Entries](https://cap.cloud.sap/docs/cds/common#protecting-certain-entries). - SAP CAPire Documentation: [Inheritance of Restrictions](https://cap.cloud.sap/docs/guides/security/authorization#inheritance-of-restrictions). +- SAP CAPire Documentation: [Authentication Enforced in Production](https://cap.cloud.sap/docs/node.js/authentication#authentication-enforced-in-production). - Common Weakness Enumeration: [CWE-862](https://cwe.mitre.org/data/definitions/862.html). -- Commoon Weakness Enumeration: [CWE-306](https://cwe.mitre.org/data/definitions/306.html). - +- Common Weakness Enumeration: [CWE-306](https://cwe.mitre.org/data/definitions/306.html). From 42de1ca6daa113f84672c3e16b792c77ecf47ebb Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Mon, 12 Aug 2024 11:51:00 -0700 Subject: [PATCH 4/8] Wrap queries and help files in their own directories --- .../{ => DefaultUserIsPrivileged}/DefaultUserIsPrivileged.md | 0 .../{ => DefaultUserIsPrivileged}/DefaultUserIsPrivileged.ql | 0 .../{ => EntityExposedWithoutAuthn}/EntityExposedWithoutAuthn.md | 0 .../{ => EntityExposedWithoutAuthn}/EntityExposedWithoutAuthn.ql | 0 .../{ => NonProductionStrategyUsed}/NonProductionStrategyUsed.ql | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename javascript/frameworks/cap/src/bad-authn-authz/{ => DefaultUserIsPrivileged}/DefaultUserIsPrivileged.md (100%) rename javascript/frameworks/cap/src/bad-authn-authz/{ => DefaultUserIsPrivileged}/DefaultUserIsPrivileged.ql (100%) rename javascript/frameworks/cap/src/bad-authn-authz/{ => EntityExposedWithoutAuthn}/EntityExposedWithoutAuthn.md (100%) rename javascript/frameworks/cap/src/bad-authn-authz/{ => EntityExposedWithoutAuthn}/EntityExposedWithoutAuthn.ql (100%) rename javascript/frameworks/cap/src/bad-authn-authz/{ => NonProductionStrategyUsed}/NonProductionStrategyUsed.ql (100%) diff --git a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md similarity index 100% rename from javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.md rename to javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md diff --git a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.ql b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.ql similarity index 100% rename from javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged.ql rename to javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.ql diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md similarity index 100% rename from javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.md rename to javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.ql b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql similarity index 100% rename from javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn.ql rename to javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql diff --git a/javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed.ql b/javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.ql similarity index 100% rename from javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed.ql rename to javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.ql From 0a062cf4cac236983605220a99544d42770ce51f Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Mon, 12 Aug 2024 15:19:18 -0700 Subject: [PATCH 5/8] Update --- .../DefaultUserIsPrivileged.md | 6 +- .../NonProductionStrategyUsed.md | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.md diff --git a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md index c88cda2d3..3d6e1c2f1 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md @@ -8,7 +8,7 @@ The user whose request cannot be verified as authenticated is represented as `cd It may be tempting to overwrite the `cds.User.default` as `cds.User.Privileged`, for ease of development. However, this may slip through production undeleted since the assignment to `cds.User.default` can be hard to detect because it may take various forms; e.g. the programmer may choose to store `cds.User` to a variable `v` and access `cds.User.default` by `v.default`. -A safer and more elegant solution is to set up a development profile and opt in to use a non-production strategy such as basic, dummy, or mocked during its use. This can be done in `package.json` of the CAP application at its project root: +A safer and more elegant solution is to set up a development profile and opt in to use a non-production strategy such as `"basic"`, `"dummy"`, or `"mocked"` during its use. This can be done in `package.json` of the CAP application at its project root: ``` json { @@ -45,6 +45,6 @@ cds.serve("all").in(app); ## References - SAP CAPire Documentation: [cds.User.default](https://cap.cloud.sap/docs/node.js/authentication#default-user). +- SAP CAPire Documentation: [cds.User.Privileged](https://cap.cloud.sap/docs/node.js/authentication#privileged-user). - SAP CAPire Documentation: [Authentication Strategies](https://cap.cloud.sap/docs/node.js/authentication#strategies). -- Common Weakness Enumeration: [CWE-862](https://cwe.mitre.org/data/definitions/862.html). -- Common Weakness Enumeration: [CWE-306](https://cwe.mitre.org/data/definitions/306.html). +- Common Weakness Enumeration: [CWE-250](https://cwe.mitre.org/data/definitions/250.html). diff --git a/javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.md b/javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.md new file mode 100644 index 000000000..e6c9c280c --- /dev/null +++ b/javascript/frameworks/cap/src/bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.md @@ -0,0 +1,63 @@ +# Non-Production Authentication Strategy Used without Profiles + +Using a non-production authentication strategy without setting up a distinct profile for development may pose allow unintended authentication and/or authorization if the application is deployed into production. + +## Recommendation + +### Isolate the use of development-level strategies to a development profile + +Use separate profiles for development and deployment and select one as needed. In this way, properties including authentication strategies can be substituted by changing a single command line option: `--profile`. For example, having the following section in the application's `package.json` states that the `"dummy"` authentication strategy must be used while `"xsuaa"`, a production-grade strategy, should be used when deployed: + +``` json +{ + "requires": { + "[dev]": { + "auth": "dummy" + }, + "[deploy]": { + "auth": "xsuaa" + } + } +} +``` + +The application can be now run in different modes depending on the `--profile` command line option: + +``` shell +$ cds serve --profile dev # Runs the application in development profile with strategy "dummy" +$ cds serve --profile deploy # Runs the application in development profile with strategy "xsuaa" +``` + +## Example + +The following CAP application states that it uses `"basic"` authentication strategy along with mocked credentials. Using the pair of username and password, an attacker can gain access to certain assets by signing in to the application. + +``` json +{ + "cds": { + "requires": { + "auth": { + "kind": "basic", + "users": { + "JohnDoe": { + "password": "JohnDoesPassword", + "roles": ["JohnDoesRole"], + "attr": {} + }, + "JaneDoe": { + "password": "JaneDoesPassword", + "roles": ["JaneDoesRole"], + "attr": {} + } + } + } + } + } +} +``` + +## References + +- Common Weakness Enumeration: [CWE-288](https://cwe.mitre.org/data/definitions/288.html). +- Common Weakness Enumeration: [CWE-798](https://cwe.mitre.org/data/definitions/798.html). +- SAP CAPire Documentation: [Authentication Strategies](https://cap.cloud.sap/docs/node.js/authentication#strategies). From 9a01d2ef07ff3855e58ba7f5d0ac88f9592e93a2 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Mon, 12 Aug 2024 15:41:15 -0700 Subject: [PATCH 6/8] Update .qlref to reflect new directory structure --- .../entities-exposed-with-cds-authz.qlref | 2 +- .../entities-exposed-with-js-authz-cds-serve.qlref | 2 +- .../entities-exposed-with-js-authz.qlref | 2 +- .../entities-exposed-with-no-authz.qlref | 2 +- .../default-is-privileged/default-is-privileged.qlref | 2 +- .../basic-authentication/basic-authentication.qlref | 2 +- .../dummy-authentication/dummy-authentication.qlref | 2 +- .../mocked-authentication/mocked-authentication.qlref | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-cds-authz/entities-exposed-with-cds-authz.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-cds-authz/entities-exposed-with-cds-authz.qlref index fc9a916e6..ad6acfbbf 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-cds-authz/entities-exposed-with-cds-authz.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-cds-authz/entities-exposed-with-cds-authz.qlref @@ -1 +1 @@ -bad-authn-authz/EntityExposedWithoutAuthn.ql \ No newline at end of file +bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz-cds-serve/entities-exposed-with-js-authz-cds-serve.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz-cds-serve/entities-exposed-with-js-authz-cds-serve.qlref index 9b981a2b8..ad6acfbbf 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz-cds-serve/entities-exposed-with-js-authz-cds-serve.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz-cds-serve/entities-exposed-with-js-authz-cds-serve.qlref @@ -1 +1 @@ -bad-authn-authz/EntityExposedWithoutAuthn.ql +bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz/entities-exposed-with-js-authz.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz/entities-exposed-with-js-authz.qlref index 9b981a2b8..ad6acfbbf 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz/entities-exposed-with-js-authz.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-js-authz/entities-exposed-with-js-authz.qlref @@ -1 +1 @@ -bad-authn-authz/EntityExposedWithoutAuthn.ql +bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-no-authz/entities-exposed-with-no-authz.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-no-authz/entities-exposed-with-no-authz.qlref index 9b981a2b8..ad6acfbbf 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-no-authz/entities-exposed-with-no-authz.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/entities-with-no-authz/entities-exposed-with-no-authz/entities-exposed-with-no-authz.qlref @@ -1 +1 @@ -bad-authn-authz/EntityExposedWithoutAuthn.ql +bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/misused-privileged-user/default-is-privileged/default-is-privileged.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/misused-privileged-user/default-is-privileged/default-is-privileged.qlref index 456949e18..6ba42e73b 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/misused-privileged-user/default-is-privileged/default-is-privileged.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/misused-privileged-user/default-is-privileged/default-is-privileged.qlref @@ -1 +1 @@ -bad-authn-authz/DefaultUserIsPrivileged.ql \ No newline at end of file +bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/basic-authentication/basic-authentication.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/basic-authentication/basic-authentication.qlref index 1c8d6cb3a..f6e8d2300 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/basic-authentication/basic-authentication.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/basic-authentication/basic-authentication.qlref @@ -1 +1 @@ -bad-authn-authz/NonProductionStrategyUsed.ql \ No newline at end of file +bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/dummy-authentication/dummy-authentication.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/dummy-authentication/dummy-authentication.qlref index cd39357ee..f6e8d2300 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/dummy-authentication/dummy-authentication.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/dummy-authentication/dummy-authentication.qlref @@ -1 +1 @@ -bad-authn-authz/NonProductionStrategyUsed.ql +bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.ql diff --git a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/mocked-authentication/mocked-authentication.qlref b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/mocked-authentication/mocked-authentication.qlref index 1c8d6cb3a..f6e8d2300 100644 --- a/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/mocked-authentication/mocked-authentication.qlref +++ b/javascript/frameworks/cap/test/queries/bad-authn-authz/nonprod-authn-strategy/mocked-authentication/mocked-authentication.qlref @@ -1 +1 @@ -bad-authn-authz/NonProductionStrategyUsed.ql \ No newline at end of file +bad-authn-authz/NonProductionStrategyUsed/NonProductionStrategyUsed.ql From df9b10b587397afc2958fc604a5442cc7b0d844a Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Tue, 13 Aug 2024 16:43:31 -0700 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Mauro Baluda --- .../DefaultUserIsPrivileged/DefaultUserIsPrivileged.md | 8 ++++---- .../EntityExposedWithoutAuthn.md | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md index 3d6e1c2f1..b1d6f5307 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/DefaultUserIsPrivileged/DefaultUserIsPrivileged.md @@ -1,14 +1,14 @@ # Default User is overwritten as privileged -The user whose request cannot be verified as authenticated is represented as `cds.User.default` internally. If this property is set to `cds.User.Privileged`, then a service may provide assets to some user who has no rights to access them. +Users that cannot be verified as authenticated are represented as `cds.User.default` internally. Setting this property to `cds.User.Privileged` may result in providing protected assets to unauthorized users. ## Recommendation ### Set up a development profile that uses non-production authentication -It may be tempting to overwrite the `cds.User.default` as `cds.User.Privileged`, for ease of development. However, this may slip through production undeleted since the assignment to `cds.User.default` can be hard to detect because it may take various forms; e.g. the programmer may choose to store `cds.User` to a variable `v` and access `cds.User.default` by `v.default`. +Overwriting `cds.User.default` as `cds.User.Privileged` for testing purposes is not recommended as such code may easily slip through production. -A safer and more elegant solution is to set up a development profile and opt in to use a non-production strategy such as `"basic"`, `"dummy"`, or `"mocked"` during its use. This can be done in `package.json` of the CAP application at its project root: +Instead, set up a development profile and opt in to use a non-production strategy such as `"basic"`, `"dummy"`, or `"mocked"` during its use. This can be done in the file `package.json` in the root folder of the CAP application: ``` json { @@ -28,7 +28,7 @@ cds serve --profile dev ## Example -Setting `cds.User.default` to `cds.User.Privileged` may happen anywhere in the application. The following is the server.js that provides the top-level definition of a CAP application, overwriting the property with the problematic class. +Setting `cds.User.default` to `cds.User.Privileged` may happen anywhere in the application. In the following example, the `server.js` file provides the top-level definition of a CAP application and overwrites the `default` user property with the `Privileged` class. ``` javascript const cds = require("@sap/cds"); diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md index fda371bd2..87a44ac4b 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md @@ -10,16 +10,16 @@ CDL provides two annotations to declare access controls `@requires` and `@restri #### Check the original CDS entity it is derived from -CDS entities may be derived from other entities by means of selection and projection. These derived definitions inherit its identical conditions for access control or can choose to override them. Therefore, in order to accurately determine what authorization an entity requires, the access control of the parent entity should be transitively inspected (that is, the inspection should go up the chain of derivation). +CDS entities may be derived from other entities by means of selection and projection. Derived definitions inherit access control conditions and optionally override them. In order to accurately determine what authorization an entity requires, the access control of the parent entity should be transitively inspected. ### Enforce authorization with JavaScript -Access control may be enforced when a handler for requests to the relevant entity or service is registered to a service. Both `cds.Service.before` and `cds.Service.on` may be used to enforce it. For example, to restrict writing to and updating an entity to certain role satisfying some requirements, the below handler registrations may be used interchangably: +Access control may be enforced when a request handler for the relevant entity or service is registered. Both `cds.Service.before` and `cds.Service.on` may be used for enforcement. For example, to restrict writing to and updating an entity to a user satisfying certain requirements, the below handler registrations may be used: ``` javascript /** - * Intercept requests to SomeEntity before access and check if the request - * is coming from a user with SomeRole and level greater than 3. + * Before serving a request to access SomeEntity, check if the request is coming from a user + * with SomeRole and level greater than 3. */ this.before(["WRITE", "UPDATE"], "SomeEntity", (req) => { (req.user.is("SomeRole") && req.user.attr.level > 3) || req.reject(403); @@ -38,7 +38,7 @@ this.on(["WRITE", "UPDATE"], "SomeEntity", (req) => { ## Examples -The following CDS definition and its JavaScript implementation imposes no authorization on SomeEntity. Note that the `OriginalEntity` from which `DerivedEntity` derives from does not control access to it, either. +The following CDS definition and its JavaScript implementation imposes no authorization on SomeEntity. Note that the `OriginalEntity` from which `DerivedEntity` derives from does not control the access either. ### db/schema.cds From 65c769544f941360d7cf788514d3d67d51d62204 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Tue, 13 Aug 2024 16:48:30 -0700 Subject: [PATCH 8/8] Minor stylistic changes --- .../EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md index 87a44ac4b..229cb1cad 100644 --- a/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md +++ b/javascript/frameworks/cap/src/bad-authn-authz/EntityExposedWithoutAuthn/EntityExposedWithoutAuthn.md @@ -14,7 +14,7 @@ CDS entities may be derived from other entities by means of selection and projec ### Enforce authorization with JavaScript -Access control may be enforced when a request handler for the relevant entity or service is registered. Both `cds.Service.before` and `cds.Service.on` may be used for enforcement. For example, to restrict writing to and updating an entity to a user satisfying certain requirements, the below handler registrations may be used: +Access control may be enforced when a request handler for the relevant entity or service is registered. Both `cds.Service.before` and `cds.Service.on` may be used for enforcement. For example, to restrict writing to and updating an entity to a user satisfying certain requirements, either one of the below handler registrations may be used: ``` javascript /** @@ -38,7 +38,7 @@ this.on(["WRITE", "UPDATE"], "SomeEntity", (req) => { ## Examples -The following CDS definition and its JavaScript implementation imposes no authorization on SomeEntity. Note that the `OriginalEntity` from which `DerivedEntity` derives from does not control the access either. +The following CDS definition and its JavaScript implementation imposes no authorization on `SomeEntity`. Note that the `OriginalEntity` from which `DerivedEntity` derives from does not control the access either. ### db/schema.cds