Added --validate option to validate session live status for FoD and SSC#977
Added --validate option to validate session live status for FoD and SSC#977rsenden merged 4 commits intofortify:dev/v3.xfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a --validate option to FoD and SSC session list commands to verify session/token status against the server (instead of relying solely on locally cached expiry information).
Changes:
- Introduced a reusable
ValidateSessionOptionMixin(--validate) and shared i18n description. - Implemented FoD session validation by calling a lightweight authenticated FoD endpoint.
- Implemented SSC session validation (including SSC version fallback) and refreshes cached SSC token expiry when available.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| fcli-core/fcli-ssc/src/main/resources/com/fortify/cli/ssc/i18n/SSCMessages.properties | Updates SSC session-list help text to describe --validate. |
| fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/access_control/helper/SSCTokenHelper.java | Adds SSC session validation logic and expiry formatting helper. |
| fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/_common/session/cli/cmd/SSCSessionListCommand.java | Wires --validate into SSC session listing and updates output fields based on live validation. |
| fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/i18n/FoDMessages.properties | Updates FoD session-list help text to describe --validate. |
| fcli-core/fcli-fod/src/main/java/com/fortify/cli/fod/_common/session/helper/oauth/FoDOAuthHelper.java | Adds FoD access token validation helper used by session listing. |
| fcli-core/fcli-fod/src/main/java/com/fortify/cli/fod/_common/session/cli/cmd/FoDSessionListCommand.java | Wires --validate into FoD session listing and updates output fields based on live validation. |
| fcli-core/fcli-common/src/main/resources/com/fortify/cli/common/i18n/FortifyCLIMessages.properties | Adds shared i18n string for the --validate option. |
| fcli-core/fcli-common/src/main/java/com/fortify/cli/common/session/cli/mixin/ValidateSessionOptionMixin.java | New common mixin implementing the --validate option behavior. |
| fcli-core/fcli-common/src/main/java/com/fortify/cli/common/action/cli/cmd/AbstractActionSignCommand.java | Minor field visibility adjustment for output helper mixin. |
| return new SessionValidationResult(false, null); | ||
| } | ||
| } | ||
| } catch ( Exception e ) { | ||
| LOG.debug("Error validating SSC session token status", e); | ||
| return new SessionValidationResult(false, null); |
There was a problem hiding this comment.
validateSession() treats any non-401/403/404 response (and any other exception) as an invalid/expired session by returning valid=false. This can misreport sessions as expired when SSC is temporarily unreachable or returns 5xx. Consider distinguishing "unknown/unverified" from "invalid" (e.g., add a third state or propagate the error) and only marking invalid on explicit authentication failures (typically 401).
| return new SessionValidationResult(false, null); | |
| } | |
| } | |
| } catch ( Exception e ) { | |
| LOG.debug("Error validating SSC session token status", e); | |
| return new SessionValidationResult(false, null); | |
| throw new FcliSimpleException("Unable to validate SSC session token status", e); | |
| } | |
| } | |
| } catch ( Exception e ) { | |
| LOG.debug("Error validating SSC session token status", e); | |
| throw new FcliSimpleException("Unable to validate SSC session token status", e); |
There was a problem hiding this comment.
moved to SSCSessionValidationHelper.checkTokenStatus() which now throws FcliSimpleException on non-401/403/404 responses, distinguishing "server unreachable/error" from "token invalid". The session list command catches this per-session and shows expired=Unknown.
| } catch ( UnexpectedHttpResponseException e ) { | ||
| return new SessionValidationResult(false, null); | ||
| } catch ( Exception e ) { | ||
| LOG.debug("Error validating SSC session by user session info endpoint", e); | ||
| return new SessionValidationResult(false, null); |
There was a problem hiding this comment.
validateSessionByInfo() returns valid=false for any UnexpectedHttpResponseException, including 5xx and potentially 403 responses. That again conflates authorization/server errors with an actually invalid token. Consider explicitly handling 401 as invalid, and treating other statuses as "unknown" (or rethrowing) so --validate doesn’t incorrectly mark sessions as expired during transient failures.
| } catch ( UnexpectedHttpResponseException e ) { | |
| return new SessionValidationResult(false, null); | |
| } catch ( Exception e ) { | |
| LOG.debug("Error validating SSC session by user session info endpoint", e); | |
| return new SessionValidationResult(false, null); | |
| } catch (UnexpectedHttpResponseException e) { | |
| if (e.getStatus() == 401) { | |
| return new SessionValidationResult(false, null); | |
| } | |
| LOG.debug("Error validating SSC session by user session info endpoint, HTTP status: {}", e.getStatus(), e); | |
| throw new FcliSimpleException("Error connecting to SSC for session validation.", e); | |
| } catch (Exception e) { | |
| LOG.debug("Error validating SSC session by user session info endpoint", e); | |
| throw new FcliSimpleException("Error connecting to SSC for session validation.", e); |
There was a problem hiding this comment.
now only HTTP 401 returns valid=false; all other HTTP errors throw FcliSimpleException
| public static final boolean validateToken(IUrlConfig urlConfig, String accessToken) { | ||
| try ( var unirest = UnirestHelper.createUnirestInstance() ) { | ||
| configureUnirest(unirest, urlConfig); | ||
| unirest.config().setDefaultHeader("Authorization", "Bearer " + accessToken); | ||
| unirest.get(FoDUrls.LOOKUP_ITEMS).queryString("limit", 1).asString(); | ||
| return true; | ||
| } catch ( Exception e ) { | ||
| LOG.debug("Error validating FoD session token", e); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
validateToken() catches all exceptions and returns false, which will mark the session as expired even for transient connectivity problems or server-side 5xx errors. Also, a 403 response typically indicates a valid token that lacks permission for this endpoint, not an expired/invalid token. Consider catching UnexpectedHttpResponseException and treating 401 as invalid, 403 as valid/unknown, and other failures as an "unknown" validation state rather than expired.
| if ( terminalDate!=null ) { | ||
| session.put("expires", SSCTokenHelper.formatExpiryDate(terminalDate)); | ||
| } |
There was a problem hiding this comment.
When --validate succeeds but SSC doesn’t return a terminal/expiry date (for example when falling back to USER_SESSION_INFO), the code sets expired to "No" but may leave expires as null from the original session summary. This results in inconsistent output (valid session but blank/unknown expiry). Consider explicitly setting expires to a consistent string like "Unknown" for valid sessions when no expiry is available.
| if ( terminalDate!=null ) { | |
| session.put("expires", SSCTokenHelper.formatExpiryDate(terminalDate)); | |
| } | |
| session.put("expires", terminalDate==null | |
| ? "Unknown" | |
| : SSCTokenHelper.formatExpiryDate(terminalDate)); |
There was a problem hiding this comment.
applySessionStatus now always writes "Unknown" when token is valid but SSC returns no terminal date (e.g. fallback via userSession/info for older SSC versions
rsenden
left a comment
There was a problem hiding this comment.
@jmadhur87, Please have a look at Copilot-generated review comments and update accordingly if applicable. Some additional comments:
- The
--validateoption doesn't make much sense for FoD, as access tokens have a fixed lifetime and cannot be revoked/updated by users. So, for FoD, this option likely creates confusion and doesn't add any value. SSCTokenHelperprovides functionality for interacting with SSC token endpoints; it doesn't know anything about fcli sessions so doesn't make sense to addvalidateSessionand related functionality in this class- Looks like the
validateSessionmethod andvalidatorconsumers have side-effects (updating the session node/description/storage), whereas based on current naming, I'd expect that these methods would just raise some error in case validation fails. Please check whether a cleaner implementation is possible - Please check for code reuse opportunities, for example, I think we already have some functionality around the SSC tokenData endpoint when creating a session
Please let me know once updated, so I can do another review.
Added new option 'validate' in fcli ssc session list and fcli fod session list to validate current status.
feat: #962