Skip to content

Revise and adjust audit logging #1443

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.

Sign up for GitHub

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

Jump to bottom
Merged
Yavor16 merged 1 commit into from
May 7, 2024
Merged

Revise and adjust audit logging #1443

Yavor16 merged 1 commit into from
May 7, 2024

Conversation

@Yavor16
Copy link
Contributor

@Yavor16 Yavor16 commented Mar 22, 2024

No description provided.

theghost5800
theghost5800 reviewed Mar 29, 2024
View reviewed changes
...b/src/main/java/org/cloudfoundry/multiapps/controller/web/security/AuthorizationChecker.java

private static void failWithStatus(HttpStatus status, String message) {
LOGGER.warn(message);
AuditLoggingProvider.getFacade()
Copy link
Contributor

@theghost5800 theghost5800 Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't log audit event anymore?

Copy link
Contributor Author

@Yavor16 Yavor16 Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The audit log logic is moved from here but still log

theghost5800
theghost5800 reviewed Apr 3, 2024
View reviewed changes
...ss/src/main/java/org/cloudfoundry/multiapps/controller/process/jobs/OrphanedDataCleaner.java
spaceClient = clientFactory.createSpaceClient(configuration.getControllerUrl(), oauthClient, Collections.emptyMap());
}

private void auditLogDeletion(T configurationData) {
Copy link
Contributor

@theghost5800 theghost5800 Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment

theghost5800
theghost5800 reviewed Apr 5, 2024
View reviewed changes
...core/src/main/java/org/cloudfoundry/multiapps/controller/core/auditlogging/AuditLogBean.java Outdated
protected DataSource dataSource;

@Bean
public AuditLoggingFacade buildAuditLoggingFacade() {
Copy link
Contributor

@theghost5800 theghost5800 Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How AuditLoggingFacade will be initialised as bean?

Copy link
Contributor Author

@Yavor16 Yavor16 Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be initialized in the xsa

Copy link
Contributor

@theghost5800 theghost5800 Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay but oss project must be also start and run without xsa part. In this way now it will always fails to initialize other beans due to missing AuditLoggingFacade bean initialization. Think for a way how to initialize this interface in oss and xsa project in the same time and override initialization in xsa project.

IvanBorislavovDimitrov
IvanBorislavovDimitrov reviewed Apr 16, 2024
View reviewed changes
theghost5800
theghost5800 reviewed Apr 19, 2024
View reviewed changes
theghost5800
theghost5800 reviewed Apr 22, 2024
View reviewed changes
...cloudfoundry/multiapps/controller/core/security/data/termination/DataTerminationService.java Outdated
private MtaConfigurationPurgerAuditLog mtaConfigurationPurgerAuditLog;

@Inject
public DataTerminationService(MtaConfigurationPurgerAuditLog mtaConfigurationPurgerAuditLog) {
Copy link
Contributor

@theghost5800 theghost5800 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be possible to convert all field inections to constructor ones? If it requires too much refactoring, then just leave it as field injection.

...n/java/org/cloudfoundry/multiapps/controller/web/resources/ConfigurationEntriesResource.java Outdated
private MtaConfigurationPurgerAuditLog mtaConfigurationPurgerAuditLog;

@Inject
public ConfigurationEntriesResource(MtaConfigurationPurgerAuditLog mtaConfigurationPurgerAuditLog) {
Copy link
Contributor

@theghost5800 theghost5800 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as for DataTerminationService

theghost5800
theghost5800 previously approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@theghost5800 theghost5800 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not forget to squash commits into 1 or smaller count with meaningful names before merge.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 7, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
16.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@Yavor16 Yavor16 merged commit 38535cf into master May 7, 2024
@Yavor16 Yavor16 deleted the add-auditlog branch May 7, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@theghost5800 theghost5800 theghost5800 approved these changes

@IvanBorislavovDimitrov IvanBorislavovDimitrov IvanBorislavovDimitrov left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Yavor16 @theghost5800 @IvanBorislavovDimitrov