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

Eas-1673 #31

Closed
wants to merge 551 commits into from
Closed

Eas-1673 #31

wants to merge 551 commits into from

Conversation

aliner-wang
Copy link
Contributor

Added logic to track and test that jira:adminPage gets tracked as a admin module.

Copy link

atlassian-cla-bot bot commented Jan 5, 2024

Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution.
If your email is listed below, please ensure that you sign the CLA with the same email address.

The following users still need to sign our CLA:
[email protected]

Already signed the CLA? To re-check, try refreshing the page.

@@ -393,7 +393,7 @@ impl<'a> ForgeModules<'a> {
.webtriggers
.binary_search_by_key(&func.key, |trigger| &trigger.function)
.is_ok();
let invokable = non_user_invokable_mod_functions.contains(func.key);
let invokable = invokable_functions.contains(func.key);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part has me a little confused in why we do what we do. With the implementation as is, the scanner will run an AuthZ scan on any function manually tracked from a non-user invokable module. But wouldn't an AuthZ scan need to be run over all user-invocable functions? Because the case missed is for functions exposed in both types of modules. So I'm wondering where an AuthZ scan is performed on user-invokable module functions? And if they're necessary?

Since the rest of the modules are presumed user-invokable, the compass modules with resolver traits should be scanned ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation(on main) currently will consider a function as invokable iff it is not on any non-invokable functions. With these changes, we want to consider a function as invokable iff it exists on an invokable module(since that more closely models reality.

If a function is exposed by both types of modules, then the latter logic has the correct semantics, as we need to scan that function, since it's user invokable. The current logic(on main) doesn't handle this case correctly and ignores that that function, even though it is invokable.

Yeah, the compass modules we talked about should be invokable(by adding it the list of invokable functions). That's the one downside of this approach; we need to track every user invokable module, whereas before we only needed to track non user invokable modules(of which there are far fewer). However, it's necessary to handle the above edge case.

@jwong101
Copy link
Contributor

jwong101 commented Jan 6, 2024

Can you rebase on top of main?

jwong101 and others added 29 commits February 12, 2024 15:40
…g modules to add to Btreeset with helper function
…finish updating into_analyzable_functions to add functions from new modules. Then add jira modules and functions associated
…ng all and new modules. Added compass_admin_page check to module function check
…ct. removed data_provider. Todo: resolve copy issue and add JSM modules
…le_functions to destructure self as refs and handle optional properties in modules that could hold functions. TODO: resolve trait requirement error.
@jwong101 jwong101 closed this Feb 29, 2024
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.

4 participants