fix(opensearch): show search results only if user has access permission to the index #5097

tsdk02 · 2024-06-24T09:40:15Z

Type of Change

Description

/search and /search/{domain} are currently checking for analytics ACL before hitting opensearch and returning results.

Additionally we have to check access permissions per index:

payment attempts/intents => PaymentRead | PaymentWrite
refunds => RefundRead | RefundWrite
disputes => DisputeRead | DisputeWrite

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

Checking access permissions to an index for a user is necessary to show results appropriately.

How did you test it?

Open the dashboard locally and hit the get_global_search_results API through the global search feature (command+k).
Search using a query (eg: USD), to get the results corresponding to respective indexes (if user has access permission)
Evaluate and verify the results

Alternatively, it can also be verified through Postman:

Hit the /search and /search/{domain} API's to retrieve the data

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

lsampras

Looks good

lsampras · 2024-06-24T12:21:11Z

crates/router/src/services/authentication.rs

@racnan Can you verify this file changes

ivor11 · 2024-06-25T09:24:20Z

crates/router/src/analytics.rs

+                        .change_context(UserErrors::InternalServerError)
+                        .change_context(OpenSearchError::UnknownError)?;
+                let permissions = role_info.get_permissions_set();
+                let accessible_indexes: Vec<_> = vec![


can this permission vector be kept in a common place for both global_search and index_search?

Yes, moved it to a consts file.

ivor11 · 2024-06-25T09:29:26Z

crates/router/src/analytics.rs

                analytics::search::msearch_results(
                    &state.opensearch_client,
                    req,
-                    &auth.merchant_account.merchant_id,
+                    &auth.merchant_id,
+                    accessible_indexes,


what happens when user has neither of the permissions? accessible_indexes = vec![]?, will msearch run on this?

Resolved this by changing the OpenMserchOutput struct to handle cases when the user has none of the permissions to access indexes.

can you share the the output of this test case

ivor11 · 2024-06-26T11:01:36Z

crates/router/src/analytics.rs

+                    .ok_or(OpenSearchError::IndexAccessNotPermittedError(index))?;
+                analytics::search::search_results(&state.opensearch_client, req, &auth.merchant_id)
+                    .await
+                    .map(ApplicationResponse::Json)
            },
            &auth::JWTAuth(Permission::Analytics),


is Analytics permission also required?

Discussed with @lsampras and he suggested to check the Analytics Permission also as of now

…ay/hyperswitch into iatapay-through-hyperswitch-cypress * 'iatapay-through-hyperswitch-cypress' of github.com:juspay/hyperswitch: feat(router): skip apple pay session call if the browser is not Safari (#5136) fix(opensearch): show search results only if user has access permission to the index (#5097) chore(version): 2024.06.27.0 feat(users): add endpoint for terminate auth select (#5135) feat(users): implemented openidconnect (#5124) feat(router): add payments manual-update api (#5045) fix(docs): open-api fix for payment response (#5103) refactor(connector): [AdyenPlatform]Throw 4xx instead of 5xx for source_balance_account (#4990) feat: realtime user analytics (#5098) refactor(connector): added amount conversion framework for cashtocode (#4857) feat(email): Add `auth_id` in email types and send `auth_id` in email URLs (#5120) refactor(connector): add amount framework to payme & Trustpay with googlePay, ApplePay for bluesnap, Noon & Trustpay (#4833) fix(connector): [BOA/CYBS] make risk information message optional (#5107) chore(version): 2024.06.25.1 fix(router): skip serialize if none for assurance_details_required in googlepay session response (#5118) refactor: separate DB queries and HTML creation for payout links (#4967) feat(router): updated `last_used_at` field for apple pay and google pay for CITs (#5087) fix(payment_methods): use existing field value of `nick_name` in db if not sent during request (#5105) chore(version): 2024.06.25.0

…ay/hyperswitch into refactor-error-handling-in-cypress * 'iatapay-through-hyperswitch-cypress' of github.com:juspay/hyperswitch: chore: clean up feat(router): skip apple pay session call if the browser is not Safari (#5136) fix(opensearch): show search results only if user has access permission to the index (#5097) chore(version): 2024.06.27.0 feat(users): add endpoint for terminate auth select (#5135) feat(users): implemented openidconnect (#5124) feat(router): add payments manual-update api (#5045) fix(docs): open-api fix for payment response (#5103) refactor(connector): [AdyenPlatform]Throw 4xx instead of 5xx for source_balance_account (#4990) feat: realtime user analytics (#5098) refactor(connector): added amount conversion framework for cashtocode (#4857) feat(email): Add `auth_id` in email types and send `auth_id` in email URLs (#5120) refactor(connector): add amount framework to payme & Trustpay with googlePay, ApplePay for bluesnap, Noon & Trustpay (#4833) fix(connector): [BOA/CYBS] make risk information message optional (#5107) chore(version): 2024.06.25.1 fix(router): skip serialize if none for assurance_details_required in googlepay session response (#5118) refactor: separate DB queries and HTML creation for payout links (#4967) feat(router): updated `last_used_at` field for apple pay and google pay for CITs (#5087) fix(payment_methods): use existing field value of `nick_name` in db if not sent during request (#5105) chore(version): 2024.06.25.0

Checking user permissions before accessing index data

1d16c37

tsdk02 requested review from lsampras and ivor11 June 24, 2024 09:40

tsdk02 self-assigned this Jun 24, 2024

tsdk02 requested review from a team as code owners June 24, 2024 09:40

tsdk02 linked an issue Jun 24, 2024 that may be closed by this pull request

fix(opensearch): show search results only if user has access to the index #5000

Closed

hyperswitch-bot bot and others added 3 commits June 24, 2024 09:40

chore: run formatter

a7cbbda

Fixed clippy errors

23be256

merged main

952fe3c

lsampras previously approved these changes Jun 24, 2024

View reviewed changes

crates/router/src/services/authentication.rs Outdated

Copy link

Member

lsampras Jun 24, 2024

Choose a reason for hiding this comment

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

@racnan Can you verify this file changes

getting user permissions using in-built functions

34d56c2

tsdk02 dismissed lsampras’s stale review via 34d56c2 June 25, 2024 08:22

chore: run formatter

1fbaedf

lsampras previously approved these changes Jun 25, 2024

View reviewed changes

ivor11 reviewed Jun 25, 2024

View reviewed changes

Merge branch 'main' into show-results-user-access

e02abe3

tsdk02 dismissed lsampras’s stale review via f3f3d83 June 25, 2024 14:16

tsdk02 requested review from a team as code owners June 25, 2024 14:16

resolved comments

b5e7678

tsdk02 force-pushed the show-results-user-access branch from eef82cf to b5e7678 Compare June 25, 2024 16:42

chore: run formatter

fe6a447

lsampras previously approved these changes Jun 26, 2024

View reviewed changes

ivor11 reviewed Jun 26, 2024

View reviewed changes

removed the NoAccess variant of SearchStatus

c11b144

tsdk02 dismissed lsampras’s stale review via c11b144 June 26, 2024 11:29

ivor11 approved these changes Jun 26, 2024

View reviewed changes

Merge branch 'main' into show-results-user-access

bcdb4fd

lsampras approved these changes Jun 26, 2024

View reviewed changes

Gnanasundari24 added this pull request to the merge queue Jun 27, 2024

Merged via the queue into main with commit 9c49ded Jun 27, 2024
11 checks passed

Gnanasundari24 deleted the show-results-user-access branch June 27, 2024 07:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(opensearch): show search results only if user has access permission to the index #5097

fix(opensearch): show search results only if user has access permission to the index #5097

tsdk02 commented Jun 24, 2024 •

edited

Loading

lsampras left a comment

lsampras Jun 24, 2024

ivor11 Jun 25, 2024

tsdk02 Jun 25, 2024

ivor11 Jun 25, 2024

tsdk02 Jun 25, 2024

ivor11 Jun 26, 2024

tsdk02 Jun 26, 2024

ivor11 Jun 26, 2024

tsdk02 Jun 26, 2024

fix(opensearch): show search results only if user has access permission to the index #5097

fix(opensearch): show search results only if user has access permission to the index #5097

Conversation

tsdk02 commented Jun 24, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

lsampras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsdk02 commented Jun 24, 2024 •

edited

Loading