-
Notifications
You must be signed in to change notification settings - Fork 740
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
Makes anonymous principal's allowedAccounts updated periodically #241
Conversation
.principal(principal) | ||
.and() | ||
.csrf().disable() | ||
.csrf() |
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.
Formatting is off here.
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 intended.
@Scheduled(initialDelay = 30000L, fixedDelay = 60000L) | ||
void updateAnonymousAccounts() { | ||
def newAnonAccounts = accountsService.getAllowedAccounts([]) | ||
anonymousAllowedAccounts.clear() |
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.
Would rather do this without the window where the list is empty.
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.
Done.
LGTM after addressing comment. |
@Autowired | ||
AccountsService accountsService | ||
|
||
List<String> anonymousAllowedAccounts = [] |
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'd make this a concurrent collection (CopyOnWriteArrayList?) since it is getting mutated in a background thread
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.
Done.
just the one comment otherwise LGTM |
LGTM, NARB |
@ajordens @cfieber @jtk54 @duftler PTAL
Tests won't pass until spinnaker/kork#40 is merged and released.