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.

Sign up for GitHub

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

Jump to bottom

[POLICY_STORE] Improvements Policy Store input validation #528

Closed
6 tasks
mkanal opened this issue Apr 9, 2024 · 8 comments
Closed
6 tasks

[POLICY_STORE] Improvements Policy Store input validation #528

mkanal opened this issue Apr 9, 2024 · 8 comments
Assignees
@dsmf @ds-kgassner
Labels
policy_store Issues regarding the IRS policy store. spill_over Issues which are not finished yet

Comments

@mkanal
Copy link
Contributor

mkanal commented Apr 9, 2024 •
edited
Loading

As product
I want to allow only valid parameters in policy store
so that we have the highest level of security

Link

Hints / Details

  • ...

Acceptance Criteria

  • Policy Store allows only BPNL numbers (no BPNA / BPNS or input with is not BPNL schema conform)
    • Registering/Update/Delete policies is only supported for BPNL
  • Registration of policy with no validUntil failes
  • Registration of policy with validUntil in the past failes
  • Registration of policy with no payload (poilicy) should fail
  • Registering a duplicate policy for same BPNL failes

Out of Scope

  • ...
@mkanal mkanal added this to IRS Apr 9, 2024
@github-project-automation github-project-automation bot moved this to inbox in IRS Apr 9, 2024
This was referenced Apr 9, 2024
Closed
Closed
@jzbmw jzbmw added the policy_store Issues regarding the IRS policy store. label Apr 11, 2024
@jzbmw jzbmw moved this from inbox to backlog in IRS Apr 12, 2024
@dsmf
Copy link
Contributor

dsmf commented Apr 17, 2024 •
edited
Loading

what about validating policyid? allowed pattern?
see https://analysiscenter.veracode.com/auth/index.jsp#ViewReportsDetailedReport:47240:1397371:34402311:34371495:34387145:::::4405498

Improper Output Neutralization for Logs CWE ID 117

  • 214 irs-api-0.0.2-SNAPSHOT-exec.jar/irs-policy-store-0.0.2-SNAPSHOT.jar
    .../PolicyStoreService.java 117
  • 146 irs-api-0.0.2-SNAPSHOT-exec.jar/irs-policy-store-0.0.2-SNAPSHOT.jar
    .../PolicyStoreService.java 143

@dsmf dsmf mentioned this issue Apr 24, 2024
Closed
@dsmf
Copy link
Contributor

dsmf commented Apr 24, 2024

relates to #555

@mkanal mkanal changed the title Improvements Policy Store input validation [POLICY_STORE] Improvements Policy Store input validation Apr 24, 2024
This was referenced Apr 24, 2024
Open
Closed
Open
@dsmf
Copy link
Contributor

dsmf commented Apr 25, 2024 •
edited
Loading

As discussed with @mkanal 25.04.2024 16:43 via MS Teams we should also validate policyId in some way. @mkanal suggested UUID here.

Current implementation therefore checks for UUID now. However it might be better to relax this to allow all alphanumeric characters plus hyphen?

@dsmf dsmf self-assigned this May 7, 2024
@dsmf dsmf moved this from backlog to wip in IRS May 7, 2024
@dsmf
Copy link
Contributor

dsmf commented May 7, 2024 •
edited
Loading

Pull Request

Outcome

Policy Store allows only BPNL numbers (no BPNA / BPNS or input with is not BPNL schema conform):

Registration of policy with no validUntil fails:

Registration of policy with validUntil in the past fails:

Registration of policy with no payload (policy) should fail:

  • payload null validation implementation and unit test
  • invalid policy json and unit test

Validation of policyId (as policyId is used as path parameter we allow only safe path parameter characters):

Registering a duplicate policy for same BPNL fails (no comparison of content required according to @mkanal):

Todos:

@dsmf dsmf mentioned this issue May 7, 2024
Merged
2 tasks
ds-jhartmann pushed a commit that referenced this issue May 7, 2024
@dsmf
chore(changelog): #528: Correct CHANGELOG.md
0415437
@ds-jhartmann ds-jhartmann mentioned this issue May 13, 2024
Merged
2 tasks
ds-jhartmann pushed a commit that referenced this issue May 13, 2024
@dsmf
remove uncommented code with TODO #528
7bef936
@dsmf dsmf unassigned mkanal May 13, 2024
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] allow only BPNL
11ba40f
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] harden tests
55b09f8
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] code formatting
573767c
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add @future to validUntil attribute
c7efaa6
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add @builder to request entities
e7f0ccd
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add validUntil validation tests
1240f77
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add and use PolicyStoreTestUtil
d3e1dcf 
in order to deduplicate common test code
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] validate policyIDs for update and delete
fa34764
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] update CHANGELOG.md
2ad9f99
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] update CHANGELOG.md
6afcdd4
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] harden ListOfPolicyIdsValidator to check for dupli…
f88f916 
…cates
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add step definition for cucumber tests for validat…
4db48e9 
…ing error response message
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add step definition for cucumber tests for validat…
aba7f8e 
…ing error response message
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] add step definitions for cucumber tests for valida…
a101fdd 
…ting error response message
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] remove obsolete uncommented test code
0f84aca
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] fix CHANGELOG.md
2711640
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] extract constraints to constants
67ea850
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] extract constraints to constants
ea44d2f
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] extract constraints to constants
94b5977
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] policyId allow "urn:uuid:" prefix
25ced4c
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] fix PMD warning
fe55a21
ds-jhartmann pushed a commit that referenced this issue May 14, 2024
@dsmf
feat(impl): [#528] relax validation of policyId to safe path paramete…
d13dfb5 
…r characters

New pattern "[a-zA-Z0-9\-_~.:]+" (safe path variable characters).
Reason: Avoid problems with too strict validation.
ds-jhartmann added a commit that referenced this issue May 14, 2024
@ds-jhartmann
Merge pull request #597 from catenax-ng/feat/528-Policy-Store-API-Inp…
060b87e 
…ut-Validation-Improve

feat(impl): [#528] Improvements Policy Store Input Validation
@mkanal mkanal added the spill_over Issues which are not finished yet label May 14, 2024
@dsmf
Copy link
Contributor

dsmf commented May 14, 2024 •
edited
Loading

all cucumber tests in main successful:

grafik
https://jira.catena-x.net/projects/TRI/issues/TRI-2017?filter=allissues

@ds-kgassner -> ready for test

@dsmf dsmf moved this from wip to test in IRS May 14, 2024
@ds-kgassner
Copy link
Contributor

Successfully tested - approved from my side

@dsmf dsmf moved this from test to review in IRS May 16, 2024
@dsmf
Copy link
Contributor

dsmf commented May 16, 2024

@mkanal -> ready for PO review

@mkanal
Copy link
Contributor Author

mkanal commented Jun 5, 2024

LGFM. PO acceptance in behalf of @jzbmw

@mkanal mkanal closed this as completed Jun 5, 2024
@mkanal mkanal moved this from review to done in IRS Jun 5, 2024
ds-jhartmann pushed a commit that referenced this issue Jun 14, 2024
@ds-ext-kmassalski
Merge pull request #528 from catenax-ng/feature/TRI-1610-change-logic…
f4f1a1c 
…-and-logs

feat(impl):[TRI-1610] change logic and add logs
This was referenced Jul 12, 2024
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dsmf dsmf

@ds-kgassner ds-kgassner

Labels
policy_store Issues regarding the IRS policy store. spill_over Issues which are not finished yet
Projects
IRS
Status: done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dsmf @jzbmw @mkanal @ds-kgassner