Skip to content
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

PAYARA-3111 Security audit service for admin console #3908

Merged
merged 15 commits into from Apr 30, 2019

Conversation

Projects
None yet
5 participants
@Cousjava
Copy link
Member

commented Apr 18, 2019

No description provided.

@Cousjava Cousjava added this to the 5.192 milestone Apr 18, 2019

@Cousjava Cousjava requested review from fturizo and Pandrex247 Apr 18, 2019

@Cousjava Cousjava self-assigned this Apr 18, 2019

@Cousjava

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

Jenkins test please

@arjantijms arjantijms requested a review from MarkWareham Apr 23, 2019

@Cousjava

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

Jenkins test please

@Cousjava Cousjava force-pushed the Cousjava:PAYARA-3111-admin-audit-servic branch from 140ac08 to 434e05c Apr 23, 2019

@Cousjava

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

Jenkins test please

@Cousjava Cousjava requested a review from arjantijms Apr 24, 2019

MeroRai and others added some commits Apr 25, 2019

* Moved the Save button to the right to keep it consistent with rest …
…of the design.

* Added Configuration Name to the page
@Cousjava

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2019

Jenkins test please

@Pandrex247
Copy link
Member

left a comment

Would be good if it was dynamic, or at least posted a warning that a restart is required to apply.
Simply enabling the service doesn't seem to have it dynamically update (from clean domain)

@Cousjava Cousjava requested a review from MeroRai Apr 29, 2019

@Service(name = "get-admin-audit-configuration")
@PerLookup
@CommandLock(CommandLock.LockType.NONE)
@ExecuteOn({RuntimeType.DAS, RuntimeType.INSTANCE})

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Apr 29, 2019

Member

Should this execute on an instance? It's a get command, surely it only wants to ask the DAS?

@MarkWareham
Copy link
Contributor

left a comment

Mine is only a surface review as I do not fully understand the architecture

if (enabledNotifiers.contains(notifier)){
attrs.put("enabled", "true");
attributes.put("enabled", "true");

This comment has been minimized.

Copy link
@MarkWareham

MarkWareham Apr 30, 2019

Contributor

Save yourself a lot of code with:
attributes.put("enabled", enabledNotifiers.contains(notifier)?"true","false")

This comment has been minimized.

Copy link
@Cousjava

Cousjava Apr 30, 2019

Author Member

I don't like ternaries, I think it makes the code less understandable

This comment has been minimized.

Copy link
@arjantijms

arjantijms Apr 30, 2019

Member
attributes.put("enabled", Boolean.toString(enabledNotifiers.contains(notifier)));
} else {
attrs.put("enabled", "false");
attributes.put("dynamic", "true");

This comment has been minimized.

Copy link
@MarkWareham

MarkWareham Apr 30, 2019

Contributor

same here

if (!command.startsWith("_")) {
return true;
}
break;

This comment has been minimized.

Copy link
@MarkWareham

MarkWareham Apr 30, 2019

Contributor

rather than the break here, you could just return false. Easier to follow the code then

@arjantijms

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

@Cousjava

Susan is about to approve the changes, but currently just waiting for you to make the command dynamic, since it isn't dynamic currently.

@Cousjava

This comment has been minimized.

Copy link
Member Author

commented Apr 30, 2019

@arjantijms @Pandrex247 I should have made it dynamic already

@MeroRai

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

@arjantijms @Pandrex247 I should have made it dynamic already

Tested it locally the command is dynamic

@arjantijms arjantijms merged commit d94fa2a into payara:master Apr 30, 2019

59 checks passed

Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.