[frontend/backend] Verify access to csv mapper in UI and API (#5954) #5979

aHenryJard · 2024-02-15T11:11:43Z

Proposed changes

[frontend] User that only has CSV Mapper (aka Access data sharing & ingestion > Manage CSV mappers) can go on /data/processing/csv_mapper using the Data menu
[frontend] User that does not have this permission cannot go on /data/processing/csv_mapper even with URL in the browser, redirected to the home page.
[frontend] Add a test on the <Security> component since it's used a lot
[backend] add protection on API woth @auth(for: [TAXIIAPI_SETCSVMAPPERS]), + added a tests to keep it protected in the future
[test]: update frontend test location to prepare the new best practice on frontend unit test + add yarn test:watch command for local test development.

Related issues

User without rights can access specific pages through URL of the platform (CSV mapper) #5954

Manual tests done locally

UI tested with:
- user that has only "Access data sharing & ingestion > Manage CSV mappers"
- user that has nothing allowed
- user that has admin but noting else (Access administration >Manage credentials, Manage marking definitions, Manage labels & Attributes)
- user that has only exploration (Access exploration >Create / Update exploration, Delete exploration, Publish exploration)
- user that has only knowledge (everything below Access knowledge)

Checklist

I consider the submitted work as finished
I tested the code for its functionality
I wrote test cases for the relevant uses case
I added/update the relevant documentation (either on github or on notion)
Where necessary I refactored code to improve the overall quality

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

aHenryJard · 2024-02-15T11:18:18Z

opencti-platform/opencti-front/src/private/components/nav/LeftBar.jsx

@@ -715,48 +715,50 @@ const LeftBar = () => {
            )}
          </MenuList>
        </Security>
-        <Security needs={[EXPLORE]}>
+        <Security needs={[EXPLORE, MODULES, KNOWLEDGE, TAXIIAPI_SETCOLLECTIONS, TAXIIAPI_SETCSVMAPPERS]}>


Here we had 2 <Security> one inside the other, and the top one was not including "MODULES, KNOWLEDGE, TAXIIAPI_SETCOLLECTIONS" that was used inside.

So it was like:

<Security needs={[EXPLORE]}> <Security needs={[MODULES, KNOWLEDGE, TAXIIAPI_SETCOLLECTIONS]}> </Security> </Security>

Now it's:

<Security needs={[EXPLORE, MODULES, KNOWLEDGE, TAXIIAPI_SETCOLLECTIONS, TAXIIAPI_SETCSVMAPPERS]}> <Security needs={[EXPLORE]}> </Security> <Security needs={[MODULES, KNOWLEDGE, TAXIIAPI_SETCOLLECTIONS, TAXIIAPI_SETCSVMAPPERS]}> </Security> </Security>

aHenryJard · 2024-02-15T11:19:43Z

opencti-platform/opencti-front/src/private/components/data/Root.jsx

@@ -109,8 +104,8 @@ const Root = () => {
          path="/dashboard/data/processing"
          render={() => (
            <Security
-              needs={[SETTINGS_SETACCESSES]}
-              placeholder={<Redirect to="/dashboard/data/processing/tasks" />}
+              needs={[KNOWLEDGE_KNUPDATE, SETTINGS_SETACCESSES, TAXIIAPI_SETCSVMAPPERS]}


inside /dashboard/data/processing some part are allowed to KNOWLEDGE_KNUPDATE and it was missing, and some part are allowed to TAXIIAPI_SETCSVMAPPERS too.

codecov · 2024-02-15T14:32:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2e7eadf) 65.44% compared to head (14d7105) 67.24%.
Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5979      +/-   ##
==========================================
+ Coverage   65.44%   67.24%   +1.79%     
==========================================
  Files         539      539              
  Lines       63059    63534     +475     
  Branches     5051     6187    +1136     
==========================================
+ Hits        41271    42724    +1453     
+ Misses      21788    20810     -978

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lndrtrbn · 2024-02-15T16:23:14Z

opencti-platform/opencti-front/src/__tests__/components/Security.test.tsx

+          <RelayEnvironmentProvider environment={environment}>
+            <AppIntlProvider settings={{ platform_language: 'auto' }}>
+              <ThemeProvider theme={createTheme()}>
+                <UserContext.Provider value={AdminContext}>
+                  <Security needs={[KNOWLEDGE_KNUPDATE, EXPLORE_EXUPDATE]}>


Well done for the tests ! It could be nice to have a helper test component like <TestApp...> that instantiate all those providers to avoid duplicating those lines that pollute the tests readings

Thanks ! This is my first React/frontend test so I did as fast as possible since it's a critical bug. Next time I will look for the reuse part :)

opencti-platform/opencti-front/src/__tests__/components/Security.test.tsx

Goumies · 2024-02-15T18:17:17Z

I have tested it locally.
On URL input, the user with the right capability can access the CSV mappers.

When navigating to CSV mappers through the sidebar (Data > Processing), the user is redirected to the login page.
Can you tell me which capabilities are required to navigate with the sidebar ?

aHenryJard · 2024-02-16T07:47:20Z

When navigating to CSV mappers through the sidebar (Data > Processing), the user is redirected to the login page. Can you tell me which capabilities are required to navigate with the sidebar ?

As far as I understand, the redirect to login occurs when there is a response "Forbidden" from API, so hard to tell which capabilities are required for everything, every API call and so on. The reference page today is https://docs.opencti.io/latest/administration/users/

For csv mapper you should only need "Access data sharing & ingestion > Manage CSV mappers" or "TAXIIAPI_SETCSVMAPPERS" in code source, if there is a login redirect then there is still an issue. I will test again.

aHenryJard · 2024-02-16T07:59:37Z

Tested again right now: with this configuration:

I have access to csv mapper:

Goumies · 2024-02-16T13:03:08Z

As we have discussed, the user only had the right capability and could not access the settings to enable Enterprise Edition.
Since I enabled it, I also added capabilities to allow access with the sidebar options.

I am checking again.

aHenryJard added 2 commits February 15, 2024 10:53

[frontend/backend] check csv mapper access.

192a449

[frontend/backend] check csv mapper access.

fb54870

aHenryJard commented Feb 15, 2024

View reviewed changes

aHenryJard linked an issue Feb 15, 2024 that may be closed by this pull request

User without rights can access specific pages through URL of the platform (CSV mapper) #5954

Closed

aHenryJard changed the title ~~Verify access to csv mapper in UI and API (#5954)~~ [frontend] Verify access to csv mapper in UI and API (#5954) Feb 15, 2024

aHenryJard added 2 commits February 15, 2024 14:28

[frontend] I have no idea on how to use RootPrivateQuery type for me.

789c5a6

[backend] add auth restriction on csv mapper query and a test coverage.

b4ff0f0

aHenryJard changed the title ~~[frontend] Verify access to csv mapper in UI and API (#5954)~~ [frontend/backend] Verify access to csv mapper in UI and API (#5954) Feb 15, 2024

aHenryJard marked this pull request as ready for review February 15, 2024 15:33

aHenryJard added the filigran team use to identify PR from the Filigran team label Feb 15, 2024

lndrtrbn reviewed Feb 15, 2024

View reviewed changes

Goumies self-requested a review February 15, 2024 17:20

Goumies reviewed Feb 15, 2024

View reviewed changes

opencti-platform/opencti-front/src/__tests__/components/Security.test.tsx Outdated Show resolved Hide resolved

aHenryJard and others added 4 commits February 16, 2024 09:01

[frontend] fix typo in test variables.

5e25ac9

[frontend] fix impossible access to csv mappers if EE active

c24a1d9

[frontend] move test next to source (best practice starting now).

04195cd

[frontend] move test next to source (best practice starting now).

14d7105

lndrtrbn approved these changes Feb 16, 2024

View reviewed changes

Goumies approved these changes Feb 16, 2024

View reviewed changes

aHenryJard merged commit 56b1782 into master Feb 16, 2024
8 checks passed

SamuelHassine deleted the issues/5954-access-to-pages branch February 16, 2024 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[frontend/backend] Verify access to csv mapper in UI and API (#5954) #5979

[frontend/backend] Verify access to csv mapper in UI and API (#5954) #5979

aHenryJard commented Feb 15, 2024 •

edited

aHenryJard Feb 15, 2024 •

edited

aHenryJard Feb 15, 2024

codecov bot commented Feb 15, 2024 •

edited

lndrtrbn Feb 15, 2024

aHenryJard Feb 15, 2024

Goumies commented Feb 15, 2024

aHenryJard commented Feb 16, 2024 •

edited

aHenryJard commented Feb 16, 2024

Goumies commented Feb 16, 2024 •

edited

[frontend/backend] Verify access to csv mapper in UI and API (#5954) #5979

[frontend/backend] Verify access to csv mapper in UI and API (#5954) #5979

Conversation

aHenryJard commented Feb 15, 2024 • edited

Proposed changes

Related issues

Manual tests done locally

Checklist

Further comments

aHenryJard Feb 15, 2024 • edited

Choose a reason for hiding this comment

aHenryJard Feb 15, 2024

Choose a reason for hiding this comment

codecov bot commented Feb 15, 2024 • edited

Codecov Report

lndrtrbn Feb 15, 2024

Choose a reason for hiding this comment

aHenryJard Feb 15, 2024

Choose a reason for hiding this comment

Goumies commented Feb 15, 2024

aHenryJard commented Feb 16, 2024 • edited

aHenryJard commented Feb 16, 2024

Goumies commented Feb 16, 2024 • edited

aHenryJard commented Feb 15, 2024 •

edited

aHenryJard Feb 15, 2024 •

edited

codecov bot commented Feb 15, 2024 •

edited

aHenryJard commented Feb 16, 2024 •

edited

Goumies commented Feb 16, 2024 •

edited