Skip to content

Conversation

@neeraj-sharma2592
Copy link
Contributor

Why make this change?

Resolves #1464 for CosmosDB.

What is this change?

Verifying the read access of anonymous users prior to transmitting the Mutation result

How was this tested?

  • Integration Tests

Copy link
Contributor

@severussundar severussundar left a comment

Choose a reason for hiding this comment

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

Need to add tests. After feedback, the format of the error message is slightly changed to include the mutation operation name as well. Please refer to this PR for the latest format

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

need tests too. @neeraj-sharma2592

@neeraj-sharma2592 neeraj-sharma2592 force-pushed the dev/neesharma/cosmos-read-permission-check-on-mutation branch from dc0d72d to 67ac50e Compare December 8, 2023 10:41
@severussundar
Copy link
Contributor

severussundar commented Dec 8, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

@severussundar severussundar left a comment

Choose a reason for hiding this comment

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

Looks good for the most part. Some suggestions around re-using helper functions and few nits in tests

Copy link
Contributor

@severussundar severussundar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

thanks for updating with this behavior and addressing feedback!

@neeraj-sharma2592 neeraj-sharma2592 enabled auto-merge (squash) December 15, 2023 16:22
@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@neeraj-sharma2592
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@neeraj-sharma2592
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@neeraj-sharma2592
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@neeraj-sharma2592
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@neeraj-sharma2592 neeraj-sharma2592 merged commit 0217d3e into main Dec 22, 2023
@neeraj-sharma2592 neeraj-sharma2592 deleted the dev/neesharma/cosmos-read-permission-check-on-mutation branch December 22, 2023 16:40
seantleonard added a commit that referenced this pull request Feb 6, 2024
…ce options #1980 (#2016)

// only diff is excluded changes from mutationtests.cs for Cosmos that
only exists in .11+. via #1893


## Why make this change?

-  Closes #1956 
- env variable replacement not happening in the `options` section of
`data-source` property in our config.

## What is this change?

- Added a DataSourceConverterFactory which will properly deserialize the
data-source section considering the _replaceEnvVariable to decide
weather to transform or not.

## How was this tested?
- [X] Unit Tests

---------

Co-authored-by: Abhishek  Kumar <[email protected]>
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.

GraphQL - Roles defined without read action returns error

5 participants