-
Notifications
You must be signed in to change notification settings - Fork 0
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
Include Context information in config response #58
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
@@ -1,4 +1,5 @@ | ||
using Bit.Core.Models.Api; | ||
using Bit.Core.Services; | ||
using Bit.Core.Settings; | ||
using Bit.Core.Utilities; | ||
|
||
|
@@ -11,6 +12,7 @@ public class ConfigResponseModel : ResponseModel | |
public ServerConfigResponseModel Server { get; set; } | ||
public EnvironmentConfigResponseModel Environment { get; set; } | ||
public IDictionary<string, object> FeatureStates { get; set; } | ||
public ContextResponseModel Context { get; set; } | ||
|
||
public ConfigResponseModel() : base("config") | ||
{ | ||
|
@@ -22,7 +24,7 @@ public ConfigResponseModel() : base("config") | |
|
||
public ConfigResponseModel( | ||
IGlobalSettings globalSettings, | ||
IDictionary<string, object> featureStates) : base("config") | ||
IDictionary<string, object> featureStates, FeatureFlagContext featureFlagContext) : base("config") | ||
{ | ||
Version = AssemblyHelpers.GetVersion(); | ||
GitHash = AssemblyHelpers.GetGitHash(); | ||
|
@@ -36,6 +38,7 @@ public ConfigResponseModel( | |
Sso = globalSettings.BaseServiceUri.Sso | ||
}; | ||
FeatureStates = featureStates; | ||
Context = new ContextResponseModel(featureFlagContext.UserId, featureFlagContext.OrganizationIds); | ||
} | ||
} | ||
|
||
|
@@ -54,3 +57,14 @@ public class EnvironmentConfigResponseModel | |
public string Notifications { get; set; } | ||
public string Sso { get; set; } | ||
} | ||
|
||
public class ContextResponseModel | ||
{ | ||
public Guid? UserId { get; set; } | ||
public Guid[] OrganizationIds { get; set; } | ||
Comment on lines
+63
to
+64
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. logic: Exposing UserId and OrganizationIds in the response could potentially be used for user enumeration attacks. Evaluate the necessity of including this information. |
||
public ContextResponseModel(Guid? userId, Guid[] organizationIds) | ||
{ | ||
UserId = userId; | ||
OrganizationIds = organizationIds; | ||
} | ||
Comment on lines
+65
to
+69
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. style: Consider adding input validation to ensure userId and organizationIds are not null or empty before assigning. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,11 @@ | ||
namespace Bit.Core.Services; | ||
|
||
public struct FeatureFlagContext | ||
{ | ||
public Guid? UserId { get; init; } | ||
public Guid[] OrganizationIds { get; init; } | ||
} | ||
|
||
public interface IFeatureService | ||
{ | ||
/// <summary> | ||
|
@@ -37,4 +43,5 @@ public interface IFeatureService | |
/// </summary> | ||
/// <returns>A dictionary of feature keys and their values.</returns> | ||
Dictionary<string, object> GetAll(); | ||
FeatureFlagContext GetFlagContext(); | ||
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. style: Consider adding XML documentation for the new |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,15 @@ public string GetStringVariation(string key, string defaultValue = null) | |
return _client.StringVariation(key, BuildContext(), defaultValue); | ||
} | ||
|
||
public FeatureFlagContext GetFlagContext() | ||
{ | ||
return new FeatureFlagContext() | ||
{ | ||
UserId = _currentContext.UserId, | ||
OrganizationIds = _currentContext.Organizations?.Select(o => o.Id).ToArray() | ||
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. style: This line may return null if _currentContext.Organizations is null. Consider using the null-coalescing operator to return an empty array instead. |
||
}; | ||
} | ||
Comment on lines
+99
to
+106
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. logic: Consider adding null checks for _currentContext and its properties to prevent potential null reference exceptions. |
||
|
||
public Dictionary<string, object> GetAll() | ||
{ | ||
var results = new Dictionary<string, object>(); | ||
|
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.
logic: Including sensitive context information in the config response may expose user data unnecessarily. Consider the security implications of this change.