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

feat: Enforce superset access check for approving apps #2023

Merged

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Jul 11, 2024

- What I did

  • Implement a verification process ensuring that only client possessing the super-set privileges or clients authorized for the specified namespaces in the enrollment request can approve it.
  • Update the minimum SDK version in pubspec.yaml to "3.0.0" to support dart 3 new features.

- How I did it

  • Refactor the "isAuthorised" method in the "abstract_verb_handler.dart" file to enforce the validation described above.
    • Remove "_isAuthorised" method and move/refactor the logic to "isAuthorised" method.
    • Introduce "_checkForNamespaceAuthorization" method, which will run the namespace authorisation check with the enrolled namespace.
    • Implement a method called "checkEnrollmentNamespaceAccess" to verify authorized namespace access according to the verb used. This method is overrides in enroll_verb_handler.dart, to validate access of namespaces in the enrollment requests against the authorized namespace access.

- How to verify it

  • Added the following unit tests:
    • A test to verify that the authorization check returns false when the client is not authorized for the requested namespace
    • A test to verify that the authorization check returns true when the client is authorized to the namespace
    • A test to verify that the authorization check returns true when the client is authorized for manage namespace
    • A test to verify that the authorization check returns true when the client is authorized to * namespaces
    • A test to verify that the authorization check returns true when the client is PKAM authentication and enrollment id is null
  • Added the following functional tests:
    • A test to assert that an authenticated connection without namespace authorization cannot approve-deny requests for that namespace
    • A test to assert that an authenticated connection without namespace authorization cannot revoke request for that namespace

- Description for the changelog

  • Enforce superset access check for approving apps

@gkc
Copy link
Contributor

gkc commented Jul 11, 2024

@sitaram-kalluri - can you add tests for these scenarios please:

  • Namespace hierarchies on enrolling side
    • Approving app has permission for "my_app" and "__manage"
    • Enrolling app is requesting permission for "data.my_app" (approver can approve)
    • Enrolling app is requesting permission for "orders.data.my_app" (approver can approve)
  • Namespace hierarchies on approving side
    • Approving app has permission for "data.my_app" and "__manage"
    • Enrolling app is requesting permission for "data.my_app" (approver can approve)
    • Enrolling app is requesting permission for "orders.data.my_app" (approver can approve)
    • Enrolling app is requesting permission for "other.my_app" (approver cannot approve)

@sitaram-kalluri sitaram-kalluri marked this pull request as draft July 11, 2024 13:01
@gkc
Copy link
Contributor

gkc commented Jul 11, 2024

One more scenario

  • approver has access to "buzz"
  • enroller requests access to "fizzbuzz" - approver can not approve
  • enroller requests access to "fizz.buzz" - approver can approve

@sitaram-kalluri sitaram-kalluri marked this pull request as ready for review July 22, 2024 05:44
@gkc
Copy link
Contributor

gkc commented Jul 24, 2024

@sitaram-kalluri LGTM; please can you run the NoPorts e2e tests locally against this at_server branch just to verify that everything is OK?

To run NoPorts e2e tests locally:

  • ensure vip.ve.atsign.zone is in your /etc/hosts pointing to 127.0.01
  • run local redis, root, and atServers using various scripts which can be found in tools/run_locally in the at_server repo
    • install redis on your machine if not already installed, and edit its config as described in the at_redis script
    • run the at_redis script
    • run the at_root script
    • run at_server -a @alice -p 10001 -s alice
    • run at_server -a @bob -p 10002 -s bob
    • run at_server -a @chuck -p 10003 -s chuck
  • create atKeys if not already done
    bin/activate_cli -a @alice -c alice -r vip.ve.atsign.zone
    bin/activate_cli -a @bob -c bob -r vip.ve.atsign.zone
    bin/activate_cli -a @chuck -c chuck -r vip.ve.atsign.zone

In a NoPorts repo clone

  • build the binaries (cd /path/to/noports/packages/dart/sshnoports; buildBinaries)
  • run an srvd
    bin/srvd -a @chuck --root-domain vip.ve.atsign.zone --ip 127.0.0.1 -v
  • run the e2e tests
    • /path/to/noports_repo/tests/e2e_all/scripts/main.sh @alice @bob @chuck -w 5 -r vip.ve.atsign.zone

@sitaram-kalluri
Copy link
Member Author

@sitaram-kalluri LGTM; please can you run the NoPorts e2e tests locally against this at_server branch just to verify that everything is OK?

To run NoPorts e2e tests locally:

* ensure vip.ve.atsign.zone is in your /etc/hosts pointing to 127.0.01

* run local redis, root, and atServers using various scripts which can be found in tools/run_locally in the at_server repo
  
  * install redis on your machine if not already installed, and edit its config as described in the `at_redis` script
  * run the `at_redis` script
  * run the `at_root` script
  * run `at_server -a @alice -p 10001 -s alice`
  * run `at_server -a @bob -p 10002 -s bob`
  * run `at_server -a @chuck -p 10003 -s chuck`

* create atKeys if not already done
  `bin/activate_cli -a @alice -c alice -r vip.ve.atsign.zone`
  `bin/activate_cli -a @bob  -c bob -r vip.ve.atsign.zone`
  `bin/activate_cli -a @chuck -c chuck -r vip.ve.atsign.zone`

In a NoPorts repo clone

* build the binaries (`cd /path/to/noports/packages/dart/sshnoports; buildBinaries`)

* run an srvd
  `bin/srvd -a @chuck --root-domain vip.ve.atsign.zone --ip 127.0.0.1 -v`

* run the e2e tests
  
  * `/path/to/noports_repo/tests/e2e_all/scripts/main.sh @alice @bob @chuck -w 5 -r vip.ve.atsign.zone`

Ran NoPorts tests locally and all tests passed. Attaching the log snippet.

### 
### 
### NoPorts e2e test run complete at 2024-07-24 18:50:31.043
### 
### Of a possible 108, 75 were not applicable (usually version constraints)
### Executed: 33  Passed: 33  Failed: 0
###########################################################
2024-07-24 18:50:31.655 | 
2024-07-24 18:50:31.657 | 

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gkc gkc merged commit ecfedc7 into trunk Jul 24, 2024
26 checks passed
@gkc gkc deleted the 2016-enhancement-enforcing-superset-access-check-for-approving-apps branch July 24, 2024 13:44
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

Successfully merging this pull request may close these issues.

Enhancement: Enforcing Superset Access Check for Approving Apps
2 participants