-
Notifications
You must be signed in to change notification settings - Fork 123
Impl/autologout #3813
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
base: main
Are you sure you want to change the base?
Impl/autologout #3813
Changes from all commits
2b8a705
8a126c6
0a0a6f3
71536df
3783a64
f600642
50b53e7
8f16fff
e21cc0e
9ed2ed6
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,7 +12,7 @@ namespace Shesha.Configuration.Security | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class SecuritySettings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Auto logoff timeout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Auto logoff timeout in minutes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public int AutoLogoffTimeout { get; set; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -57,5 +57,20 @@ public class SecuritySettings | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [Display(Name = "Default endpoint access", Description = "Used for 'Inherited' endpoint access")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public RefListPermissionedAccess DefaultEndpointAccess { get; set; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Enable logout when user is inactive | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public bool LogoutWhenUserInactive { get; set; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Warning time before logout in seconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public int LogoutWarningSeconds { get; set; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Enable logout when browser is closed/hidden | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public bool LogoutWhenBrowserClosed { get; set; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+60
to
+74
Contributor
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. 🛠️ Refactor suggestion Add validation attributes for new settings to enforce safe ranges Prevent misconfiguration and align with UI constraints. using System.ComponentModel.DataAnnotations;
@@
- public bool LogoutWhenUserInactive { get; set; }
+ public bool LogoutWhenUserInactive { get; set; }
@@
- public int LogoutWarningSeconds { get; set; }
+ [Range(10, 300)]
+ public int LogoutWarningSeconds { get; set; }
@@
- public bool LogoutWhenBrowserClosed { get; set; }
+ public bool LogoutWhenBrowserClosed { get; set; }Optionally also validate: - public int AutoLogoffTimeout { get; set; }
+ [Range(0, 1440)]
+ public int AutoLogoffTimeout { get; set; }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,8 @@ public class SheshaFrameworkModule : SheshaModule | |
| #endif | ||
| }; | ||
|
|
||
| public bool SkipAppWarmUp { get; set; } | ||
|
|
||
|
Comment on lines
+46
to
+47
Contributor
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. 💡 Verification agent ❓ Verification inconclusiveWhere is SkipAppWarmUp set? Verify usage across the solution. Ensure this flag is actually set from startup/config and not left at default false. Run: 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find where the flag is assigned or read
rg -nP '\bSkipAppWarmUp\b' -C2Length of output: 0 🏁 Script executed: #!/bin/bash
# Display the first 200 lines of SheshaFrameworkModule for inspection
sed -n '1,200p' shesha-core/src/Shesha.Framework/SheshaFrameworkModule.csLength of output: 100 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate SheshaFrameworkModule.cs in the repo
fd --type f --name 'SheshaFrameworkModule.cs'
# Search for any assignments or usages of SkipAppWarmUp throughout the codebase
rg -nP '\bSkipAppWarmUp\b' -C2Length of output: 250 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate SheshaFrameworkModule.cs in the repository
find . -type f -name '*SheshaFrameworkModule.cs'
# Search for any references or assignments of SkipAppWarmUp across the codebase
rg -n 'SkipAppWarmUp' -C2Length of output: 0 Wire up or remove SkipAppWarmUp flag 🤖 Prompt for AI Agents |
||
| public SheshaFrameworkModule() | ||
| { | ||
| } | ||
|
|
@@ -129,7 +131,10 @@ public override void Initialize() | |
| s.DefaultAccountLockoutSeconds.WithDefaultValue(300 /* 5 minutes */); | ||
| s.SecuritySettings.WithDefaultValue(new SecuritySettings | ||
| { | ||
| AutoLogoffTimeout = 0, | ||
| AutoLogoffTimeout = 15, // 15 minutes default, | ||
| LogoutWhenUserInactive = false, | ||
| LogoutWarningSeconds = 60, | ||
| LogoutWhenBrowserClosed = false, | ||
| UseResetPasswordViaEmailLink = true, | ||
| ResetPasswordEmailLinkLifetime = 60, | ||
| UseResetPasswordViaSmsOtp = true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| using Shesha.DynamicEntities.Dtos; | ||
| using Shesha.Extensions; | ||
| using Shesha.Metadata; | ||
| using Shesha.Reflection; | ||
| using Shesha.Utilities; | ||
| using System.Collections.Generic; | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
@@ -158,8 +159,11 @@ public bool Validate(object obj, string propertyName, object? value, List<Valida | |
| } | ||
| break; | ||
| case DataTypes.Number: | ||
| var b = double.TryParse(value?.ToString(), out double val); | ||
| var stringValue = value?.ToString(); | ||
| if (string.IsNullOrWhiteSpace(stringValue) && propInfo != null && propInfo.IsNullable()) | ||
| return true; | ||
|
|
||
| var b = double.TryParse(stringValue, out double val); | ||
| if (!b) | ||
|
Comment on lines
+162
to
167
Contributor
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. Blank numeric strings can bypass Required; treat whitespace as null and enforce Required. Current logic returns true for whitespace when the target property is nullable, but it doesn’t honor propConfig.Required for “blank” inputs (value != null). This can let required numeric fields pass as blank. Apply: - var stringValue = value?.ToString();
- if (string.IsNullOrWhiteSpace(stringValue) && propInfo != null && propInfo.IsNullable())
- return true;
-
- var b = double.TryParse(stringValue, out double val);
+ var stringValue = value?.ToString();
+ if (string.IsNullOrWhiteSpace(stringValue))
+ {
+ if (propConfig.Required && !propConfig.Suppress)
+ {
+ validationResult.Add(new ValidationResult(hasMessage
+ ? propConfig.ValidationMessage
+ : $"Property '{friendlyName}' is required."));
+ return false;
+ }
+ if (propInfo != null && propInfo.IsNullable())
+ return true;
+ }
+
+ var b = double.TryParse(stringValue, out double val);Optional: consider culture-aware parsing to avoid ambiguity with decimals/thousands (requires var b = double.TryParse(stringValue, NumberStyles.Float | NumberStyles.AllowThousands, CultureInfo.CurrentCulture, out double val);🤖 Prompt for AI Agents |
||
| { | ||
| validationResult.Add(new ValidationResult($"Property '{friendlyName}' should be in a number format")); | ||
|
|
||
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.
Querying the wrong repository likely excludes most item types
Repository points to ConfigurationItem (the concrete type bound to the CRUD base), while export needs all ConfigurationItemBase derivatives. Use _itemsRepository to include all item types.
📝 Committable suggestion
🤖 Prompt for AI Agents