-
Notifications
You must be signed in to change notification settings - Fork 2
Add authentication/authorization related PoCs #107
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
Merged
Merged
Conversation
This file contains hidden or 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
1. Authentication strategy is `dummy` 2. Authentication strategy is `basic` with hardcoded credentials 3. Authentication strategy is `mocked` with hardcoded credentials 4. `cds.User.default = cds.User.Privileged;`
mbaluda
approved these changes
Apr 9, 2024
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.
It's a good set of tests!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR contributes
This PR adds several PoCs pertaining to authentication or authorization issues, as well as model tests to make sure that the model classes correctly capture the relevant portions of code.
nonprod-authn-strategy
Level: Error, Precision: High.
This category is about mistakes due to using development-only authentication strategies, assuming code-scanning is called before the application goes into production.
basic/mocked/dummy-authentication
Level: Error, Precision: High.
package.json
.misused-privileged-user
This category pertains to the mistakes caused by misusing the
cds.User.Privileged
class in a couple of ways. Since this class bypasses all means of authorization, developers should be prudent in ways it is used.default-is-privileged
Level: Error, Precision: High.
The cds.User.default property, initially
cds.User.Anonymous
, is overwritten tocds.User.Privileged
at some execution point of the application. This models the scenario where the developer overwrote the property for convenience during development but forgot to put it back to the default class.dynamically-generated-privileged
Level: Error, Precision: Low.
A user of
cds.User.Privileged
is created and used somewhere in the application in an inappropriate manner. Particularly, its misuse allows for elevated access rights of a user to its maximum.privileged-user.js
contains a custom authentication strategy that creates a privileged user on login.service1.js
contains five transactions that reads from various entities with a privileged user:this.on("send1", ...)
: Not an error. It SELECTs from its own entity that does not require authorization.this.on("send2", ...)
: Is an error. It SELECTs from its own entity that requires authorization.this.on("send3", ...)
: Not an error. It SELECTs from a locally available entity that does not require authorization.this.on("send4", ...)
: Is an error. It SELECTs from a locally available entity that requires authorization.this.on("send5", ...)
: Is eligible for a warning. It SELECTs from a remote entity of which authorization strategy is unknown. Treating this as an error may result in many false positives, so we overapproximate that the remote entity does restrict access rights in some way and issue a warning instead.entities-with-no-authn
Level: Warning, Precision: High.
When a CAP application chooses to use a production-level authentication strategy, all of its entities, services, actions, and functions are "locked down" to require 'authenticated-user' as its access control. Therefore, a warning should be issued whenever a CAP declaration without an access control is encountered.
Alternatively, if the aformentioned issue is present but
cds.env.requires.auth.restrict_all_services
is set to false in the application'spackage.json
, then an error should be raised.entities-exposed-with-no-authz
All entities, services, actions, and functions are not secured. Every occurrences of such should raise an alarm.
entities-exposed-with-cds-authz
All entities, services, actions, and functions require authentication via @restrict or @requires annotations on the CDS file that declares them (Note that event declarations cannot be qualified with neither of both). As a result, no alerts should be produced on this application.
entities-exposed-with-js-authz
All entities, services, actions, and functions require authentication either via:
Service.before
handler registration that checks access rights and rejects the request, orService.on
handler registration that checks access rights and executes business logic or rejects the request on the result of the authorization.Hence, for similar reasons for
entities-exposed-with-cds-authz
, no alerts should be produced on this application.Model tests
privilegeduser
privilegeduser.js
contains true-positive cases where a privileged user is created to access an entity, with three variations in how it is configured:req.user
as a privileged user. The user is initialized by a constructor call toCustomPrivilegedUser1
, which is equivalent tocds.User.Privileged
since it has a constanttrue
function as itsis
instance method used for authorization.cds.User.Privileged
.CustomPrivilegedUser1
.req-user-authz
Extensive collection of all combinations involving
this.before
andthis.on
handler registrations written for authentization and authorization, which is similar to those appearing in the exampledynamically-generated-privileged
said above.Future work
Implement queries that correctly finds all true positive cases but avoids all false positive ones.