Skip to content

Conversation

@asloobq
Copy link
Contributor

@asloobq asloobq commented May 29, 2025

No description provided.

router.route().failureHandler(new GenericFailureHandler());

final boolean enableAuditLog = true;
final AuditParams auditParams = new AuditParams();
Copy link
Contributor

Choose a reason for hiding this comment

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

why? isn't it better to have 1 less parm in constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed this by overloading function 'handleWithAudit' to accept fewer arguments

Comment on lines 197 to 209
router.get(Endpoints.CLOUD_ENCRYPTION_KEYS_RETRIEVE.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleCloudEncryptionKeysRetrieval), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.SITES_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleSiteRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.KEY_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleKeyRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.KEY_ACL_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleKeyAclRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.KEY_KEYSET_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleKeysetRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.KEY_KEYSET_KEYS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleKeysetKeyRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.SALT_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleSaltRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.CLIENTS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleClientRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.CLIENT_SIDE_KEYPAIRS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleClientSideKeypairRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.SERVICES_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleServiceRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.SERVICE_LINKS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleServiceLinkRefresh), auditParams, enableAuditLog, Role.OPERATOR));
router.get(Endpoints.OPERATORS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handleOperatorRefresh), auditParams, enableAuditLog, Role.OPTOUT_SERVICE));
router.get(Endpoints.PARTNERS_REFRESH.toString()).handler(auth.handleWithAudit(attestationMiddleware.handle(this::handlePartnerRefresh), auditParams, enableAuditLog, Role.OPTOUT_SERVICE));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is getting harder to read. We should move away from the pattern where handlers have inner handlers. The next handler is already available on the routing context. It would be better to split it up like this

router.get(Endpoints.SITES_REFRESH.toString())
        .handler(auth.handleWithAudit(auditParams, enableAuditLog, Role.OPERATOR))
        .handler(attestationMiddleware::handle)
        .handler(this::handleSiteRefresh);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Perhaps we do it as part of continuous improvement


private final FileSystem fileSystem;

private static final String OPERATOR_TYPE = "operator_type";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe name it something like OPERATOR_TYPE_QUERY_PARAM_NAME otherwise it's a bit confusing 🤔

@asloobq asloobq merged commit fb092e8 into main Jun 4, 2025
4 checks passed
@asloobq asloobq deleted the aaq-UID2-5481-audit-log branch June 4, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants