Skip to content
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

Insufficient authorization query #100

Open
mbaluda opened this issue Mar 6, 2024 · 5 comments
Open

Insufficient authorization query #100

mbaluda opened this issue Mar 6, 2024 · 5 comments
Assignees

Comments

@mbaluda
Copy link
Contributor

mbaluda commented Mar 6, 2024

Relevant sources:

@mbaluda mbaluda self-assigned this Mar 6, 2024
@jeongsoolee09
Copy link
Contributor

jeongsoolee09 commented Mar 12, 2024

A standard library query: User-controlled bypass of security check. Related CWEs:

  • Common Weakness Enumeration: CWE-807.
  • Common Weakness Enumeration: CWE-290.

@jeongsoolee09
Copy link
Contributor

jeongsoolee09 commented Mar 27, 2024

Authorization enforcement in Node.js:

service CustomerService @(requires: 'authenticated-user'){
  entity Orders @(restrict: [
    { grant: ['READ','WRITE'], to: 'admin' },
  ]){/*...*/}
  entity Approval @(restrict: [
    { grant: 'WRITE', where: '$user.level > 2' }
  ]){/*...*/}
}

is equivalent to

const cds = require('@sap/cds')
cds.serve ('CustomerService') .with (function(){
  this.before ('*', req =>
   req.user.is('authenticated') || req.reject(403)
  )
  this.before (['READ', 'CREATE'], 'Orders', req =>
    req.user.is('admin') || req.reject(403)
  )
  this.before ('*', 'Approval', req =>
    req.user.attr.level > 2 || req.reject(403)
  )
})

Note that before registration function is used to intercept the request and check the request parameter for authorization before it gets processed.

@jeongsoolee09
Copy link
Contributor

jeongsoolee09 commented Mar 27, 2024

Sample CDS @restrict annotations scraped from CAPire:

entity Orders @(restrict: [
    { grant: 'READ', to: 'Auditor', where: 'AuditBy = $user' }
]) {/*...*/}

entity Reviews @(restrict: [
    { grant:['READ', 'WRITE'], to: ['Reviewer', 'Customer'] }
]) {/*...*/}

entity Orders @(restrict: [
    { grant: ['READ','WRITE'], to: 'Admin' },
    { grant: 'READ', where: 'buyer = $user' }
]) {/*...*/}

entity Orders @(restrict: [
    { grant: 'READ', to: 'Auditor', where: 'country = $user.country' },
    { grant: ['READ','WRITE'], where: 'CreatedBy = $user' },
]) {/*...*/}

service CatalogService {
    entity Products as projection on db.Products { ... }
    actions {
        @(requires: 'Admin')
        action addRating (stars: Integer);
    }
    function getViewsCount @(restrict: [{ to: 'Admin' }]) () returns Integer;
}

service CustomerService @(requires: 'authenticated-user') {
    entity Products @(restrict: [
        { grant: 'READ' },
        { grant: 'WRITE', to: 'Vendor' },
        { grant: 'addRating', to: 'Customer'}
    ]) {/*...*/}
    actions {
        action addRating (stars: Integer);
    }
    entity Orders @(restrict: [
        { grant: '*', to: 'Customer', where: 'CreatedBy = $user' }
    ]) {/*...*/}
    action monthlyBalance @(requires: 'Vendor') ();
}

Key takeaways:

  • @restrict attaches to entities (or function parameters), whereas @requires qualifies actions and services.
  • @restrict entries typically contains grant attribute, while where and to can be used as needed.

So checking if the service is protected in terms of authentication requires:

  • Authz on Service
    • CDS: check presence of @requires annotation
    • JS: check presence of this.before('*', req => { req.user check or req.reject })
  • Authz on Entity
    • CDS: check presence of @restrict annotation
    • JS: check presence of this.before(something, entityName, req => { req.user check or req.reject })
  • Authz on Action
    • CDS: check presence of @requires annotation
    • JS: check presence of this.before(actionName, req => { req.user check or req.reject })
  • Authz on a Function Parameter
    • CDS: check presence of @restrict annotation
    • JS: check presence of this.before(functionName, optionalBoundEntity?, req => { req.user check or req.reject })

@jeongsoolee09
Copy link
Contributor

My misunderstanding: The difference between @restrict and @requires isn't in the kind of CDL elements they can be attached to; rather, it's in in the granularity. @requires can only specify the roles that are permitted to access the resource, whereas @restrict can also specify the roles with access levels with where.

@jeongsoolee09
Copy link
Contributor

The services / entities / actions / functions can be protected with JS implementation code as well.

Protection with this.before

The conditional here lacks a meaningful success branch. Examples:

this.before("*", (req) => {
  if (!req.user.is("authenticated")) {
    req.reject(403);
  }
});

this.before("*", (req) => {
  req.user.is("authenticated") ? undefined : req.reject(403);
});

this.before("*", (req) => {
  req.user.is("authenticated") && true || req.reject(403);
});

Dual:

this.before("*", (req) => {
  if (req.user.is("authenticated")) {
  } else {
    req.reject(403);
  }
});

this.before("*", (req) => {
  !req.user.is("authenticated") ? req.reject(403) : undefined;
});

this.before("*", (req) => {
  (!req.user.is("authenticated") && req.reject(403)) || false;
});

Protection with this.on

The conditional here has both success branch and failure branch. Examples:

this.on("*", (req) => {
  if (req.user.is("authenticated")) {
    /* Do something */
  } else req.reject(403);
});

this.on("*", (req) => {
  req.user.is("authenticated") ? /* Do something */ true : req.reject(403);
});

this.on("*", (req) => {
  (req.user.is("authenticated") && /* Do something */ true) || req.reject(403);
});

Dual:

this.on("*", (req) => {
  if (!req.user.is("authenticated")) {
    req.reject(403);
  } else {
    /* Do something */
  }
});

this.on("*", (req) => {
  !req.user.is("authenticated") ? req.reject(403) : /* Do something */ true;
});

this.on("*", (req) => {
  (!req.user.is("authenticated") && req.reject(403)) || /* Do something */ true;
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants