mattcasters commented on code in PR #8631:
URL: https://github.com/apache/hop/pull/8631#discussion_r4153848978
##########
core/src/main/java/org/apache/hop/core/security/HopSecurity.java:
##########
@@ -70,6 +72,22 @@ public static HopSecurityContext getContext() {
return context != null ? context : HopSecurityContext.unrestricted();
}
+ /**
+ * The name to record in the audit fields of a file, such as the user who
created or last modified
+ * a pipeline or workflow.
+ *
+ * <p>Only an authenticated context yields a name. The desktop runs
unrestricted under the {@link
+ * HopSecurityContext#ANONYMOUS_USERNAME} placeholder, and neither that
placeholder nor the
+ * operating system account name says anything about who edited the file, so
those cases return an
+ * empty result and leave whatever the file already holds untouched.
+ *
+ * @return the authenticated user name, or empty when no real user is known
+ */
+ public static Optional<String> getAuditUsername() {
+ HopSecurityContext context = getContext();
+ return context.isAuthenticated() ? Optional.of(context.getUsername()) :
Optional.empty();
Review Comment:
**[bug]** `isAuthenticated()` is not "a real person edited this file".
Desktop privilege simulation starts from an unrestricted `anonymous` context;
`HopSecurityPrivilegeMode.createEffective` rewrites that username to `session`
and `forUser` clears `unrestricted`, so this method returns `session`. New file
and save then call `stampCreated` / `stampModified` and write `session` into
`created_user` / `modified_user`. A later normal desktop save only updates the
user inside `ifPresent`, so the stand-in stays while `modified_date` moves
forward and the save is attributed to `session`.
**Suggestion:** Treat only a real principal as an audit name. Return empty
for the privilege-mode stand-in (or consult the login-time base context, which
stays unrestricted on the desktop even while a role is assumed). Add a test
whose provider returns
`HopSecurityPrivilegeMode.createEffective(HopSecurityContext.unrestricted(),
HopRole.READ_ONLY)` and assert the user fields stay `-`.
##########
engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java:
##########
@@ -111,6 +111,15 @@ public class WorkflowMeta extends AbstractMeta
@HopMetadataProperty(inline = true)
protected WorkflowMetaInfo info;
+ /**
+ * The version string for the workflow.
+ *
+ * @deprecated since 2.20, for removal. The label has to be set and updated
by hand and says
+ * nothing that the version control system holding the file does not
already say more
+ * reliably. It is still read from and written to the file, so no
existing value is lost
+ * before it is removed.
+ */
+ @Deprecated(since = "2.20", forRemoval = true)
Review Comment:
**[suggestion]** Scheduling `workflow_version` (and
`PipelineMetaInfo.pipelineVersion`) for removal drops a signal the git plugin
still uses. `GitGuiPlugin.showWorkflowFileDiff` / `showPipelineFileDiff` set
the version to `git: ` plus the commit id on the diff copies, and
`DrawDiffOnActionExtensionPoint` / `DrawDiffOnTransformExtensionPoint` paint
markers only when that value starts with `git`. The new javadoc says the field
says nothing version control does not already say; those in-memory diff tabs
use it as their marker. Separately, class-level Lombok `@Getter` / `@Setter`
does not copy `@Deprecated` (no `lombok.copyableAnnotations`), so
`getWorkflowVersion` / `setWorkflowVersion` stay undeprecated and
`WorkflowDialog` plus the git plugin will not warn.
`PipelineMetaInfo.pipelineStatus` has the same accessor gap. The hand-written
`PipelineMeta.getPipelineVersion` / `setPipelineVersion` are deprecated
correctly.
**Suggestion:** Keep both version fields until the diff markers live on
another attribute. Deprecate the workflow accessors explicitly, for example a
field-level `@Getter(onMethod_ = @__(@Deprecated(since = "2.20", forRemoval =
true)))` and matching `@Setter` (same for pipeline status), so call sites warn
before any removal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
