-
Notifications
You must be signed in to change notification settings - Fork 1
changed to use secure operations to restrict operations #25
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
Conversation
| await tableOperations.DeleteRecordAsync(record, cancellationToken); | ||
| SecureTableOperations<T> tableOperations = new(connection); | ||
| object primaryKey = tableOperations.BaseOperations.GetPrimaryKeys(record).First(); | ||
| await tableOperations.DeleteRecordWhereAsync(HttpContext.User, $"{PrimaryKeyField} = {{0}}", cancellationToken, primaryKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we changing this call?
Shouldn't we use
await tableOperations.DeleteRecordAsync(HttpContext.User, record, cancellationToken);
| SecureTableOperations<T> tableOperations = new(connection); | ||
|
|
||
| T? result = tableOperations.NewRecord(); | ||
| T? result = tableOperations.BaseOperations.NewRecord(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for cleaner operation let's add NewRecord to SecureTableOperations
| TableOperations<T> tableOperations = new(connection); | ||
| string tableName = tableOperations.TableName; | ||
| SecureTableOperations<T> tableOperations = new(connection); | ||
| string tableName = tableOperations.BaseOperations.TableName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| string tableName = tableOperations.BaseOperations.TableName; | |
| string tableName = tableOperations.TableName; |
Let's handle this in SecureTableOperations There are ways to have optional parameters in an interface definition., That would allow SecureTableOeprationsto beITableOperations` which I think makes this cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were avoiding doing that since adding a ClaimsPrincipal would change basically every function footprint away from the ITableOperations interface.
In anycase, I'll put up a branch with the required change for indirection being taken care of by SecureTableOperations .
This PR implements claim checking as an additional security measure to filter data through a tableoperations wrapper in model controllers.
JIRA Ticket(s)
Additional Support for: DIG-21
Ticket: DIG-22
Required PRs