-
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-12358] New Verified Organization Domain SSO Detail endpoint #4838
base: main
Are you sure you want to change the base?
Changes from all commits
39cd275
abb6b53
98484e9
4e5a9e6
aac397b
0cc2512
c22656e
fec6fb5
2702d3e
b3e1398
9103f91
7fd0ead
43598a2
d95eab7
04f0e2c
b15e0a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
๏ปฟusing Bit.Core.Models.Api; | ||
using Bit.Core.Models.Data.Organizations; | ||
|
||
namespace Bit.Api.AdminConsole.Models.Response.Organizations; | ||
|
||
public class VerifiedOrganizationDomainSsoDetailResponseModel : ResponseModel | ||
{ | ||
public VerifiedOrganizationDomainSsoDetailResponseModel(VerifiedOrganizationDomainSsoDetail data) | ||
: base("verifiedOrganizationDomainSsoDetails") | ||
{ | ||
Check warning on line 10 in src/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs Codecov / codecov/patchsrc/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs#L9-L10
|
||
if (data is null) | ||
{ | ||
throw new ArgumentNullException(nameof(data)); | ||
Check warning on line 13 in src/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs Codecov / codecov/patchsrc/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs#L12-L13
|
||
} | ||
|
||
DomainName = data.DomainName; | ||
OrganizationIdentifier = data.OrganizationIdentifier; | ||
OrganizationName = data.OrganizationName; | ||
} | ||
public string DomainName { get; } | ||
public string OrganizationIdentifier { get; } | ||
public string OrganizationName { get; } | ||
Check warning on line 22 in src/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs Codecov / codecov/patchsrc/Api/AdminConsole/Models/Response/Organizations/VerifiedOrganizationDomainSsoDetailResponseModel.cs#L16-L22
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
๏ปฟusing Bit.Api.Models.Response; | ||
|
||
namespace Bit.Api.AdminConsole.Models.Response.Organizations; | ||
|
||
public class VerifiedOrganizationDomainSsoDetailsResponseModel( | ||
IEnumerable<VerifiedOrganizationDomainSsoDetailResponseModel> data, | ||
string continuationToken = null) | ||
: ListResponseModel<VerifiedOrganizationDomainSsoDetailResponseModel>(data, continuationToken); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
๏ปฟnamespace Bit.Core.Models.Data.Organizations; | ||
|
||
public class VerifiedOrganizationDomainSsoDetail | ||
{ | ||
public VerifiedOrganizationDomainSsoDetail() | ||
{ | ||
} | ||
|
||
public VerifiedOrganizationDomainSsoDetail(Guid organizationId, string organizationName, string domainName, | ||
string organizationIdentifier) | ||
{ | ||
OrganizationId = organizationId; | ||
OrganizationName = organizationName; | ||
DomainName = domainName; | ||
OrganizationIdentifier = organizationIdentifier; | ||
} | ||
|
||
public Guid OrganizationId { get; init; } | ||
public string OrganizationName { get; init; } | ||
public string DomainName { get; init; } | ||
public string OrganizationIdentifier { get; init; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,29 @@ | |
return ssoDetails; | ||
} | ||
|
||
public async Task<IEnumerable<VerifiedOrganizationDomainSsoDetail>> GetVerifiedOrganizationDomainSsoDetailsAsync(string email) | ||
{ | ||
var domainName = new MailAddress(email).Host; | ||
Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/OrganizationDomainRepository.cs Codecov / codecov/patchsrc/Infrastructure.EntityFramework/Repositories/OrganizationDomainRepository.cs#L99-L100
|
||
|
||
using var scope = ServiceScopeFactory.CreateScope(); | ||
var dbContext = GetDatabaseContext(scope); | ||
return await (from o in dbContext.Organizations | ||
from od in o.Domains | ||
join s in dbContext.SsoConfigs on o.Id equals s.OrganizationId into sJoin | ||
from s in sJoin.DefaultIfEmpty() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want the same join on policy here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Policy is unneeded for this. The original join to Policy was about checking if SSO was required...which isn't a restraint we need for this query. |
||
where od.DomainName == domainName | ||
&& o.Enabled | ||
&& s.Enabled | ||
&& od.VerifiedDate != null | ||
Comment on lines
+108
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is missing some of the conditions from its MSSQL equivalent, like directly checking that the SSO policy is enabled for the organization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I looked over what I had in the sproc compared to what was being used, and I see that it doesn't actually care about the policy of |
||
select new VerifiedOrganizationDomainSsoDetail( | ||
o.Id, | ||
o.Name, | ||
od.DomainName, | ||
o.Identifier)) | ||
.AsNoTracking() | ||
.ToListAsync(); | ||
} | ||
Check warning on line 119 in src/Infrastructure.EntityFramework/Repositories/OrganizationDomainRepository.cs Codecov / codecov/patchsrc/Infrastructure.EntityFramework/Repositories/OrganizationDomainRepository.cs#L102-L119
|
||
|
||
public async Task<Core.Entities.OrganizationDomain?> GetDomainByIdOrganizationIdAsync(Guid id, Guid orgId) | ||
{ | ||
using var scope = ServiceScopeFactory.CreateScope(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
CREATE PROCEDURE [dbo].[VerifiedOrganizationDomainSsoDetails_ReadByEmail] | ||
@Email NVARCHAR(256) | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON | ||
|
||
DECLARE @Domain NVARCHAR(256) | ||
|
||
SELECT @Domain = SUBSTRING(@Email, CHARINDEX( '@', @Email) + 1, LEN(@Email)) | ||
|
||
SELECT | ||
O.Id AS OrganizationId, | ||
O.Name AS OrganizationName, | ||
O.Identifier AS OrganizationIdentifier, | ||
OD.DomainName | ||
FROM [dbo].[OrganizationView] O | ||
INNER JOIN [dbo].[OrganizationDomainView] OD ON O.Id = OD.OrganizationId | ||
LEFT JOIN [dbo].[Ssoconfig] S ON O.Id = S.OrganizationId | ||
WHERE OD.DomainName = @Domain | ||
AND O.Enabled = 1 | ||
AND OD.VerifiedDate IS NOT NULL | ||
AND S.Enabled = 1 | ||
END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
CREATE OR ALTER PROCEDURE [dbo].[VerifiedOrganizationDomainSsoDetails_ReadByEmail] | ||
@Email NVARCHAR(256) | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON | ||
|
||
DECLARE @Domain NVARCHAR(256) | ||
|
||
SELECT @Domain = SUBSTRING(@Email, CHARINDEX( '@', @Email) + 1, LEN(@Email)) | ||
|
||
SELECT | ||
O.Id AS OrganizationId, | ||
O.Name AS OrganizationName, | ||
O.Identifier AS OrganizationIdentifier, | ||
OD.DomainName | ||
FROM [dbo].[OrganizationView] O | ||
INNER JOIN [dbo].[OrganizationDomainView] OD ON O.Id = OD.OrganizationId | ||
LEFT JOIN [dbo].[Ssoconfig] S ON O.Id = S.OrganizationId | ||
WHERE OD.DomainName = @Domain | ||
AND O.Enabled = 1 | ||
AND OD.VerifiedDate IS NOT NULL | ||
AND S.Enabled = 1 | ||
END | ||
GO |
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.
Can you explain to me why these need
AllowAnonymous
? I can see it on the method above too, but I don't immediately understand why. Claiming domains is done from authorized sessions in the admin console, isn't it?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.
This is a get for retrieving the claimed domains. It is used when attempting to log in via SSO. The user is not logged in at this point. This endpoint is retrieved in order to try to determine the org and domain needed for SSO login.