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

Admin monitor message: separation between content and display #3416

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/util/HttpResponses.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public static HttpResponse errorJSON(@Nonnull String message) {
* @param data The data.
* @return {@code this} object.
*
* @since TODO
* @since 2.119
*/
public static HttpResponse errorJSON(@Nonnull String message, @Nonnull Map<?,?> data) {
return new JSONObjectResponse(data).error(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* Potential exceptions are the dev environment, if someone disable the wizard or
* the administrator put an empty string on the configuration page.
*
* @since TODO
* @since 2.119
*/
@Extension
@Symbol("rootUrlNotSet")
Expand All @@ -52,15 +52,13 @@ public String getDisplayName() {
@Override
public boolean isActivated() {
JenkinsLocationConfiguration loc = JenkinsLocationConfiguration.get();
return loc != null && (
loc.getUrl() == null || !UrlHelper.isValidRootUrl(loc.getUrl())
);
return loc.getUrl() == null || !UrlHelper.isValidRootUrl(loc.getUrl());
}

// used by jelly to determined if it's a null url or invalid one
@Restricted(NoExternalUse.class)
public boolean isUrlNull(){
JenkinsLocationConfiguration loc = JenkinsLocationConfiguration.get();
return loc != null && loc.getUrl() == null;
return loc.getUrl() == null;
Copy link
Member

Choose a reason for hiding this comment

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

If you don't need the null check on loc here than can you remove it from isActivated as well? Looks like it is @CheckForNull so I think you want to add it back.

Copy link
Contributor Author

@Wadeck Wadeck May 2, 2018

Choose a reason for hiding this comment

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

It's already the case for isActivated and the JenkinsLocationConfiguration.get() is @Nonnull.

Copy link
Member

Choose a reason for hiding this comment

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

I was looking at an old version, this looks good to me. Sorry for the noise!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ public String getDisplayName() {

@Override
public boolean isActivated() {
return Jenkins.getInstance().getCrumbIssuer() == null;
return Jenkins.get().getCrumbIssuer() == null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,24 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core">
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<div class="alert alert-warning">
<form method="post" action="${rootURL}/${it.url}/disable">
<f:submit value="${%Dismiss}"/>
</form>
<j:set var="actionAnchor">
<a href="${rootURL}/configure">${%actionUrlContent}</a>
</j:set>

<j:choose>
<j:when test="${it.isUrlNull()}">
${%urlIsNull(rootURL)}
${%urlIsNull}
</j:when>
<j:otherwise>
${%urlIsInvalid(rootURL)}
${%urlIsInvalid}
</j:otherwise>
</j:choose>
<p />
<j:out value="${%actionToTake(actionAnchor)}"/>
</div>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
urlIsNull=Jenkins root URL is empty but is required for the proper operation of many Jenkins features like email notifications, \
PR status update, and environment variables such as <code>BUILD_URL</code>.<br />\
Please provide an accurate value in <a href="{0}/configure">Jenkins configuration</a>.
PR status update, and environment variables such as <code>BUILD_URL</code>.
urlIsInvalid=Jenkins root URL seems to be invalid. It is required for the proper operation of many Jenkins features like email notifications, \
PR status update, and environment variables such as <code>BUILD_URL</code>.<br />\
Please provide an accurate value in <a href="{0}/configure">Jenkins configuration</a>.
PR status update, and environment variables such as <code>BUILD_URL</code>.
actionToTake=Please provide an accurate value in {0}.
actionUrlContent=Jenkins configuration
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,19 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core">
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<div class="alert alert-warning">
${%warningMessage(rootURL)}
<form method="post" action="${rootURL}/${it.url}/disable">
Copy link
Member

Choose a reason for hiding this comment

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

Is allowing this monitor to be dismissed important enough for a changelog entry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think as it's already possible by passing throught the configuration page, it's just a shortcut

<f:submit value="${%Dismiss}"/>
</form>
<j:set var="referenceAnchor">
<a href="https://jenkins.io/redirect/csrf-protection" target="_blank">${%referenceUrlContent}</a>
</j:set>
<j:set var="actionAnchor">
<a href="${rootURL}/configureSecurity">${%actionUrlContent}</a>
</j:set>
<j:out value="${%warningMessage(referenceAnchor)}"/>
<p />
<j:out value="${%actionMessage(actionAnchor)}"/>
</div>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
warningMessage=You have not configured the CSRF issuer. This could be a security issue. \
For more information, please refer to <a href="https://jenkins.io/redirect/csrf-protection" target="_blank">this page</a>. \
<br /> \
You can change the current configuration using the Security section <a href="{0}/configureSecurity">CSRF Protection</a>.
For more information, please refer to {0}.
actionMessage=You can change the current configuration using the Security section {0}.
referenceUrlContent=this page
actionUrlContent=CSRF Protection
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

warningMessage=L''emittente CSRF non � stata configurata. Ci� potrebbe costituire \
un problema di sicurezza. \
Per ulteriori informazioni, fare riferimento a <a href="https://jenkins.io/redirect/csrf-protection" target="_blank">questa pagina</a>. \
<br /> \
� possibile modificare la configurazione corrente nella sezione \
<a href="{0}/configureSecurity">Protezione CSRF</a> della categoria Sicurezza.
Per ulteriori informazioni, fare riferimento a {0}.
actionMessage=� possibile modificare la configurazione corrente nella sezione {0} della categoria Sicurezza.
referenceUrlContent=questa pagina
actionUrlContent=Protezione CSRF