-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-14406] Security Task Notifications #5344
Open
nick-livefront
wants to merge
75
commits into
main
Choose a base branch
from
vault/pm-14406/security-task-notifications
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
75 commits
Select commit
Hold shift + click to select a range
69da981
[PM-14378] Introduce GetCipherPermissionsForOrganization query for Da…
shane-melton c85a930
[PM-14378] Introduce GetCipherPermissionsForOrganization method for E…
shane-melton db5bd64
[PM-14378] Add integration tests for new repository method
shane-melton a03ddee
[PM-14378] Introduce IGetCipherPermissionsForUserQuery CQRS query
shane-melton 532dd07
[PM-14378] Introduce SecurityTaskOperationRequirement
shane-melton 5c38328
[PM-14378] Introduce SecurityTaskAuthorizationHandler.cs
shane-melton 19a1814
[PM-14378] Introduce SecurityTaskOrganizationAuthorizationHandler.cs
shane-melton df3e424
[PM-14378] Register new authorization handlers
shane-melton 78ea8b5
[PM-14378] Formatting
shane-melton 4d80238
[PM-14378] Add unit tests for GetCipherPermissionsForUserQuery
shane-melton b40d144
[PM-15378] Cleanup SecurityTaskAuthorizationHandler and add tests
shane-melton ca15550
[PM-14378] Add tests for SecurityTaskOrganizationAuthorizationHandler
shane-melton bcf3210
[PM-14378] Formatting
shane-melton 021634c
[PM-14378] Update date in migration file
shane-melton d222923
Merge branch 'main' into vault/pm-14378/security-task-auth-handler
shane-melton 6eb33cf
Merge branch 'main' into vault/pm-14378/security-task-auth-handler
shane-melton b10df9b
[PM-14378] Add missing awaits
shane-melton 3a04e88
Added bulk create request model
gbubemismith 9cef90b
Merge branch 'main' into vault/pm-14378/security-task-auth-handler
shane-melton d00b25b
Created sproc to create bulk security tasks
gbubemismith 3198ea7
Renamed tasks to SecurityTasksInput
gbubemismith 4fabb8a
Added create many implementation for sqlserver and ef core
gbubemismith ef37e9d
removed trailing comma
gbubemismith 0edb923
created ef implementatin for create many and added integration test
gbubemismith 2684901
Refactored request model
gbubemismith 0adb3e3
Refactored request model
gbubemismith ba848ce
created create many tasks command interface and class
gbubemismith 74b25da
Merge commit 'refs/pull/5039/head' of https://github.com/bitwarden/se…
gbubemismith 7738ce4
added security authorization handler work temp
gbubemismith 67f3215
Added the implementation for the create manys tasks command
gbubemismith d2bed09
Added comment
gbubemismith 6aaba7e
Changed return to return list of created security tasks
gbubemismith 04c7c56
Registered command
gbubemismith 6df6132
Completed bulk create action
gbubemismith e3352f4
Added unit tests for the command
gbubemismith e7abb09
removed hard coded table name
gbubemismith 8868b3c
Fixed lint issue
gbubemismith 72b121d
Added JsonConverter attribute to allow enum value to be passed as string
gbubemismith fff7707
Fixed merge conflicts
gbubemismith 770fa34
Removed makshift security task operations
gbubemismith 239a191
Fixed references
gbubemismith 714666b
Removed old migration
gbubemismith a3ffb5f
Rebased
gbubemismith a99afb6
[PM-14378] Introduce GetCipherPermissionsForOrganization query for Da…
shane-melton 4cfbd3d
[PM-14378] Introduce GetCipherPermissionsForOrganization method for E…
shane-melton 2481ca1
[PM-14378] Add unit tests for GetCipherPermissionsForUserQuery
shane-melton b4e890a
Completed bulk create action
gbubemismith c1c7f3f
Fixed conflicts
gbubemismith 6740471
Merge branch 'main' into vault/PM-14381
gbubemismith 897285f
bumped migration version
gbubemismith e6c7722
Fixed lint issue
gbubemismith 6f2e408
initial commit of `CipherOrganizationPermission_GetManyByUserId`
nick-livefront 8022148
create queries to get all of the security tasks that are actionable b…
nick-livefront c239f19
rename query
nick-livefront 945f2db
return the user's email from the query as well
nick-livefront 2652f80
Add email notification for at-risk passwords
nick-livefront 7585106
Merge branch 'main' of https://github.com/bitwarden/server into vault…
nick-livefront b067638
Removed complex sql data type in favour of json string
gbubemismith 7e82d62
Merge branch 'main' into vault/PM-14381
gbubemismith e59946c
Register IGetTasksForOrganizationQuery
gbubemismith 2b4c9a9
Fixed lint issue
gbubemismith 28fb1ed
add push notification for security tasks
nick-livefront 2ef25f5
Removed tasks grouping
gbubemismith b4ac978
Fixed linting
gbubemismith a9629c8
Removed unused code
gbubemismith 05561f1
Removed unused code
gbubemismith fbc86fb
update entity framework to match stored procedure plus testing
nick-livefront 5664944
update date of migration and remove orderby
nick-livefront 6cce985
add push service to security task controller
nick-livefront cadd8d4
Merge branch 'vault/PM-14381' of https://github.com/bitwarden/server …
nick-livefront 9ac2bc6
rename `SyncSecurityTasksCreated` to `SyncNotification`
nick-livefront 447a60a
remove duplicate return
nick-livefront fb725b2
remove unused directive
nick-livefront 1c9037d
remove unneeded new notification type
nick-livefront b43b131
use `createNotificationCommand` to alert all platforms
nick-livefront File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
src/Api/Vault/Models/Request/BulkCreateSecurityTasksRequestModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using Bit.Core.Vault.Models.Api; | ||
|
||
namespace Bit.Api.Vault.Models.Request; | ||
|
||
public class BulkCreateSecurityTasksRequestModel | ||
{ | ||
public IEnumerable<SecurityTaskCreateRequest> Tasks { get; set; } | ||
} |
61 changes: 61 additions & 0 deletions
61
src/Core/MailTemplates/Handlebars/Layouts/SecurityTasks.html.hbs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
{{#>FullUpdatedHtmlLayout}} | ||
<table border="0" cellpadding="0" cellspacing="0" width="100%" | ||
style="background-color: #175DDC;padding-top:25px;padding-bottom:15px;"> | ||
<tr> | ||
<td align="center" valign="top" width="70%" class="templateColumnContainer"> | ||
<table border="0" cellpadding="0" cellspacing="0" width="100%" | ||
style="padding-left:30px; padding-right: 5px; padding-top: 20px;"> | ||
<tr> | ||
<td | ||
style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 24px; color: #ffffff; line-height: 32px; font-weight: 500; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;"> | ||
{{OrgName}} has identified {{TaskCount}} critical login{{#if TaskCountPlural}}s{{/if}} that require{{#unless | ||
TaskCountPlural}}s{{/unless}} a | ||
password change | ||
</td> | ||
</tr> | ||
</table> | ||
</td> | ||
<td align="right" valign="bottom" class="templateColumnContainer" style="padding-right: 15px;"> | ||
<img width="140" height="140" align="right" valign="bottom" | ||
style="width: 140px; height:140px; font-size: 0; vertical-align: bottom; text-align: right;" alt='' | ||
src='https://assets.bitwarden.com/email/v1/business-warning.png' /> | ||
</td> | ||
</tr> | ||
</table> | ||
|
||
{{>@partial-block}} | ||
|
||
<table width="100%" style="display:table; background-color: #FBFBFB; vertical-align: middle; padding:30px" border="0" | ||
cellpadding="0" cellspacing="0" valign="middle"> | ||
<tr> | ||
<td width="70%" class="footer-text" style="padding-right: 20px;"> | ||
<table align="left" border="0" cellpadding="0" cellspacing="0"> | ||
<tr> | ||
<td | ||
style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-style: normal; font-weight: 400; font-size: 16px; line-height: 24px;"> | ||
<p | ||
style="margin: 0; padding: 0; margin-bottom: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-style: normal; font-weight: 600; font-size: 20px; line-height: 28px;"> | ||
We’re here for you!</p> | ||
If you have any questions, search the Bitwarden <a | ||
style="text-decoration: none; color: #175DDC; font-weight: 600;" | ||
href="https://bitwarden.com/help/">Help</a> site or <a | ||
style="text-decoration: none; color: #175DDC; font-weight: 600;" | ||
href="https://bitwarden.com/contact/">contact us</a>. | ||
</td> | ||
</tr> | ||
</table> | ||
</td> | ||
<td width="30%"> | ||
<table align="right" valign="bottom" class="footer-image" border="0" cellpadding="0" cellspacing="0" | ||
style="padding-left: 40px;"> | ||
<tr> | ||
<td> | ||
<img width="94" height="77" src="https://assets.bitwarden.com/email/v1/chat.png" | ||
style="width: 94px; height: 77px;" alt="" /> | ||
</td> | ||
</tr> | ||
</table> | ||
</td> | ||
</tr> | ||
</table> | ||
{{/FullUpdatedHtmlLayout}} |
12 changes: 12 additions & 0 deletions
12
src/Core/MailTemplates/Handlebars/Layouts/SecurityTasks.text.hbs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{{#>FullTextLayout}} | ||
{{OrgName}} has identified {{TaskCount}} critical login{{#if TaskCountPlural}}s{{/if}} that require{{#unless | ||
TaskCountPlural}}s{{/unless}} a | ||
password change | ||
|
||
{{>@partial-block}} | ||
|
||
We’re here for you! | ||
If you have any questions, search the Bitwarden Help site or contact us. | ||
- https://bitwarden.com/help/ | ||
- https://bitwarden.com/contact/ | ||
{{/FullTextLayout}} |
28 changes: 28 additions & 0 deletions
28
src/Core/MailTemplates/Handlebars/SecurityTasksNotification.html.hbs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{{#>SecurityTasksHtmlLayout}} | ||
<table width="100%" border="0" style="display: block; padding: 30px;" align="center" cellpadding="0" cellspacing="0"> | ||
<tr> | ||
<td | ||
style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-style: normal; font-weight: 400; font-size: 16px; line-height: 24px;"> | ||
Keep you and your organization's data safe by changing passwords that are weak, reused, or have been exposed in a | ||
data breach. | ||
</td> | ||
</tr> | ||
<tr> | ||
<td | ||
style="padding-top: 24px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-style: normal; font-weight: 400; font-size: 16px; line-height: 24px;"> | ||
Launch the Bitwarden extension to review your at-risk passwords. | ||
</td> | ||
</tr> | ||
</table> | ||
<table width="100%" border="0" cellpadding="0" cellspacing="0" | ||
style="display: table; width:100%; padding-bottom: 35px; text-align: center;" align="center"> | ||
<tr> | ||
<td display="display: table-cell"> | ||
<a href="{{ReviewPasswordsUrl}}" clicktracking=off target="_blank" | ||
style="display: inline-block; color: #ffffff; text-decoration: none; text-align: center; cursor: pointer; border-radius: 999px; background-color: #175DDC; border-color: #175DDC; border-style: solid; border-width: 10px 20px; margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;"> | ||
Review at-risk passwords | ||
</a> | ||
</td> | ||
</tr> | ||
</table> | ||
{{/SecurityTasksHtmlLayout}} |
8 changes: 8 additions & 0 deletions
8
src/Core/MailTemplates/Handlebars/SecurityTasksNotification.text.hbs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{{#>SecurityTasksHtmlLayout}} | ||
Keep you and your organization's data safe by changing passwords that are weak, reused, or have been exposed in a data | ||
breach. | ||
|
||
Launch the Bitwarden extension to review your at-risk passwords. | ||
|
||
Review at-risk passwords ({{{ReviewPasswordsUrl}}}) | ||
{{/SecurityTasksHtmlLayout}} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
namespace Bit.Core.Models.Mail; | ||
|
||
public class SecurityTaskNotificationViewModel : BaseMailModel | ||
{ | ||
public string OrgName { get; set; } | ||
|
||
public int TaskCount { get; set; } | ||
|
||
public bool TaskCountPlural => TaskCount != 1; | ||
|
||
public string ReviewPasswordsUrl => $"{WebVaultUrl}/browser-extension-prompt"; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
using Bit.Core.Context; | ||
using Bit.Core.Exceptions; | ||
using Bit.Core.Utilities; | ||
using Bit.Core.Vault.Authorization.SecurityTasks; | ||
using Bit.Core.Vault.Commands.Interfaces; | ||
using Bit.Core.Vault.Entities; | ||
using Bit.Core.Vault.Enums; | ||
using Bit.Core.Vault.Models.Api; | ||
using Bit.Core.Vault.Repositories; | ||
using Microsoft.AspNetCore.Authorization; | ||
|
||
namespace Bit.Core.Vault.Commands; | ||
|
||
public class CreateManyTasksCommand : ICreateManyTasksCommand | ||
{ | ||
private readonly IAuthorizationService _authorizationService; | ||
private readonly ICurrentContext _currentContext; | ||
private readonly ISecurityTaskRepository _securityTaskRepository; | ||
|
||
public CreateManyTasksCommand( | ||
ISecurityTaskRepository securityTaskRepository, | ||
IAuthorizationService authorizationService, | ||
ICurrentContext currentContext) | ||
{ | ||
_securityTaskRepository = securityTaskRepository; | ||
_authorizationService = authorizationService; | ||
_currentContext = currentContext; | ||
} | ||
|
||
/// <inheritdoc /> | ||
public async Task<ICollection<SecurityTask>> CreateAsync(Guid organizationId, | ||
IEnumerable<SecurityTaskCreateRequest> tasks) | ||
{ | ||
if (!_currentContext.UserId.HasValue) | ||
{ | ||
throw new NotFoundException(); | ||
} | ||
|
||
var tasksList = tasks?.ToList(); | ||
|
||
if (tasksList is null || tasksList.Count == 0) | ||
{ | ||
throw new BadRequestException("No tasks provided."); | ||
} | ||
|
||
var securityTasks = tasksList.Select(t => new SecurityTask | ||
{ | ||
OrganizationId = organizationId, | ||
CipherId = t.CipherId, | ||
Type = t.Type, | ||
Status = SecurityTaskStatus.Pending | ||
}).ToList(); | ||
|
||
// Verify authorization for each task | ||
foreach (var task in securityTasks) | ||
{ | ||
await _authorizationService.AuthorizeOrThrowAsync( | ||
_currentContext.HttpContext.User, | ||
task, | ||
SecurityTaskOperations.Create); | ||
} | ||
|
||
return await _securityTaskRepository.CreateManyAsync(securityTasks); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 was unsure of what properties to set here, this felt like a good starting point for the task notifications.
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.
Great question, these seem reasonable to me though!
❓ Is this creating multiple notifications per user per task? I haven't dug too deep into the PR yet, but this is what we're going for now and we'll likely need to update this to include the
TaskId
in theNotification
(this is being added in PM-14590 [Server] Modify Notification database tableThere 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.
Currently this sends a single notification (push and email) to an individual. Are you saying that we want to have a notification for each security task that the user can action on? That will be kind of noisy, no? X number of emails for each applicable task.
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.
There will be a notification row in our backend database associated to a task for a variety of reasons to make management of the tasks better, but that is not to imply that communications such as push, email, etc. are 1:1 -- it would be per bulk operation.