Skip to content

Commit

Permalink
Hook Prefixes
Browse files Browse the repository at this point in the history
Administrator should limit which URLs can Webhooks (especially test
ones) hit by defining a list of allowed URL prefixes.
  • Loading branch information
rvansa committed Jun 2, 2021
1 parent 2d16fdb commit f316f44
Show file tree
Hide file tree
Showing 17 changed files with 624 additions and 203 deletions.
47 changes: 42 additions & 5 deletions src/main/java/io/hyperfoil/tools/horreum/api/HookService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import io.hyperfoil.tools.horreum.JsonAdapter;
import io.hyperfoil.tools.horreum.entity.alerting.Change;
import io.hyperfoil.tools.horreum.entity.json.AllowedHookPrefix;
import io.hyperfoil.tools.horreum.entity.json.Hook;
import io.hyperfoil.tools.horreum.entity.json.Run;
import io.hyperfoil.tools.horreum.entity.json.Test;
import io.hyperfoil.tools.yaup.json.Json;
import io.quarkus.hibernate.orm.panache.PanacheEntityBase;
import io.quarkus.panache.common.Page;
import io.quarkus.panache.common.Sort;
import io.quarkus.security.identity.SecurityIdentity;
Expand All @@ -19,6 +21,7 @@
import org.jboss.logging.Logger;

import javax.annotation.PostConstruct;
import javax.annotation.security.PermitAll;
import javax.annotation.security.RolesAllowed;
import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
Expand All @@ -36,6 +39,7 @@
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.net.URL;
Expand All @@ -44,6 +48,7 @@
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

@Path("/api/hook")
@Consumes({MediaType.APPLICATION_JSON})
Expand Down Expand Up @@ -147,7 +152,7 @@ public void newChange(Change.Event changeEvent) {
tellHooks(Change.EVENT_NEW, run.testid, changeEvent.change);
}

@RolesAllowed(Roles.ADMIN)
@RolesAllowed({ Roles.ADMIN, Roles.TESTER})
@POST
@Transactional
public Response add(Hook hook){
Expand All @@ -172,7 +177,7 @@ public Response add(Hook hook){
}
}

@RolesAllowed(Roles.ADMIN)
@RolesAllowed({ Roles.ADMIN, Roles.TESTER})
@GET
@Path("{id}")
public Hook get(@PathParam("id")Integer id){
Expand All @@ -181,7 +186,7 @@ public Hook get(@PathParam("id")Integer id){
}
}

@RolesAllowed(Roles.ADMIN)
@RolesAllowed({ Roles.ADMIN, Roles.TESTER})
@DELETE
@Path("{id}")
@Transactional
Expand Down Expand Up @@ -222,16 +227,48 @@ public List<Hook> list(@QueryParam("limit") Integer limit,
}
}

// @RolesAllowed(Roles.ADMIN)
@RolesAllowed({ Roles.ADMIN, Roles.TESTER})
@GET
@Path("test/{id}")
public List<Hook> hooks(@PathParam("id") Integer testId) {
try (@SuppressWarnings("unused") CloseMe closeMe = sqlService.withRoles(em, identity)) {
if (testId != null) {
return Hook.list("target", testId);
} else {
return Hook.listAll();
throw new WebApplicationException("No test ID set.", 400);
}
}
}

@PermitAll
@GET
@Path("prefixes")
public List<AllowedHookPrefix> allowedPrefixes() {
return AllowedHookPrefix.listAll();
}

@RolesAllowed(Roles.ADMIN)
@Consumes("text/plain")
@POST
@Path("prefixes")
@Transactional
public AllowedHookPrefix addPrefix(String prefix) {
try (@SuppressWarnings("unused") CloseMe closeMe = sqlService.withRoles(em, identity)) {
AllowedHookPrefix p = new AllowedHookPrefix();
// FIXME: fetchival stringifies the body into JSON string :-/
p.prefix = Util.destringify(prefix);
em.persist(p);
return p;
}
}

@RolesAllowed(Roles.ADMIN)
@DELETE
@Path("prefixes/{id}")
@Transactional
public void deletePrefix(@PathParam("id") Long id) {
try (@SuppressWarnings("unused") CloseMe closeMe = sqlService.withRoles(em, identity)) {
AllowedHookPrefix.delete("id", id);
}
}
}
13 changes: 1 addition & 12 deletions src/main/java/io/hyperfoil/tools/horreum/api/RunService.java
Original file line number Diff line number Diff line change
Expand Up @@ -967,18 +967,7 @@ public Response trash(@PathParam("id") Integer id, @QueryParam("isTrashed") Bool
@Transactional
public Response updateDescription(@PathParam("id") Integer id, String description) {
// FIXME: fetchival stringifies the body into JSON string :-/
return updateRun(id, run -> run.description = destringify(description));
}

private static String destringify(String str) {
if (str == null || str.isEmpty()) {
return str;
}
if (str.charAt(0) == '"' && str.charAt(str.length() - 1) == '"') {
return str.substring(1, str.length() - 1);
} else {
return str;
}
return updateRun(id, run -> run.description = Util.destringify(description));
}

private Response updateRun(Integer id, Consumer<Run> consumer) {
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/io/hyperfoil/tools/horreum/api/Util.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.hyperfoil.tools.horreum.api;

class Util {
static String destringify(String str) {
if (str == null || str.isEmpty()) {
return str;
}
if (str.charAt(0) == '"' && str.charAt(str.length() - 1) == '"') {
return str.substring(1, str.length() - 1);
} else {
return str;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.hyperfoil.tools.horreum.entity.json;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.validation.constraints.NotNull;

import io.quarkus.hibernate.orm.panache.PanacheEntityBase;

@Entity(name = "allowedhookprefix")
public class AllowedHookPrefix extends PanacheEntityBase {
@Id
@GeneratedValue
public Long id;

@NotNull
public String prefix;
}
28 changes: 28 additions & 0 deletions src/main/resources/db/changeLog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1211,5 +1211,33 @@
$$ LANGUAGE plpgsql STABLE;
</createProcedure>
</changeSet>

<changeSet id="24" author="rvansa">
<createTable tableName="allowedhookprefix">
<column name="id" type="bigint">
<constraints primaryKey="true" nullable="false" />
</column>
<column name="prefix" type="text">
<constraints nullable="false" />
</column>
</createTable>
<!-- Only admin and test owners can both view and modify webhooks. -->
<sql>
GRANT SELECT, INSERT, DELETE, UPDATE ON TABLE allowedhookprefix TO "${quarkus.datasource.username}";
ALTER TABLE allowedhookprefix ENABLE ROW LEVEL SECURITY;
CREATE POLICY prefix_select ON allowedhookprefix FOR SELECT USING(true);
CREATE POLICY prefix_insert ON allowedhookprefix FOR INSERT WITH CHECK (has_role('admin'));
CREATE POLICY prefix_update ON allowedhookprefix FOR UPDATE USING (has_role('admin'));
CREATE POLICY prefix_delete ON allowedhookprefix FOR DELETE USING (has_role('admin'));

ALTER TABLE hook ENABLE ROW LEVEL SECURITY;
CREATE POLICY hook_policies ON hook
USING (has_role('admin') OR exists(
SELECT 1 FROM test WHERE (type = 'change/new' OR type = 'run/new') AND target = test.id AND can_view(test.access, test.owner, test.token)
));
CREATE POLICY hook_write_check ON hook
WITH CHECK (exists(SELECT 1 FROM allowedhookprefix ahp WHERE left(url, length(ahp.prefix)) = ahp.prefix));
</sql>
</changeSet>
</databaseChangeLog>

114 changes: 114 additions & 0 deletions webapp/src/components/HookUrlSelector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import React, { useEffect, useState, MutableRefObject } from 'react'
import { useDispatch } from 'react-redux'

import {
Dropdown,
DropdownItem,
DropdownToggle,
FormGroup,
InputGroup,
TextInput,
} from '@patternfly/react-core';

import { AllowedHookPrefix } from '../domain/hooks/reducers';
import { fetchPrefixes } from '../domain/hooks/api'

import { alertAction } from '../alerts'

function isValidUrl(url: string) {
try {
new URL(url);
return true;
} catch (_) {
return false;
}
}

type HookUrlSelectorProps = {
active: boolean,
value: string,
setValue(value: string): void,
isDisabled?: boolean,
isReadOnly?: boolean,
setValid?(valid: boolean): void,
extraCheck?(value: string): string | boolean
}

function HookUrlSelector(props: HookUrlSelectorProps) {
const dispatch = useDispatch();
const [prefixes, setPrefixes] = useState<AllowedHookPrefix[]>([{ id: -1, prefix: ""}])
useEffect(() => {
if (props.active) {
fetchPrefixes().then(setPrefixes,
e => dispatch(alertAction('FETCH_HOOK_PREFIXES', "Failed to fetch allowed hook prefixes", e)))
}
}, [dispatch, props.active])

const isUrlValid = isValidUrl(props.value)
const isUrlAllowed = prefixes.some(p => props.value.startsWith(p.prefix))
const extraCheckResult = props.extraCheck ? props.extraCheck(props.value) : true

const [dropdownOpen, setDropdownOpen] = useState(false)

const ref = React.useRef<any>()

return (
<FormGroup
label="Webhook URL"
validated={ isUrlValid && isUrlAllowed && extraCheckResult === true ? "default" : "error"}
isRequired={true}
fieldId="url"
helperText="URL (with protocol) for POST callback"
helperTextInvalid={ !isUrlValid ? "URL cannot be parsed." : !isUrlAllowed ? "URL does not start with any of the allowed prefixes." : extraCheckResult}
>
<InputGroup>
{ !props.isReadOnly && <Dropdown
onSelect={ event => {
if (event && event.currentTarget) {
if (props.setValid) {
props.setValid(true)
}
props.setValue(event.currentTarget.innerText)
}
setDropdownOpen(false)
if (ref.current) {
ref.current.focus()
}
}}
toggle={
<DropdownToggle
onToggle={ setDropdownOpen }
isDisabled={ props.isDisabled } >
Pick URL prefix
</DropdownToggle>
}
isOpen={ dropdownOpen }
dropdownItems={ prefixes.map((p, i) => (
<DropdownItem key={i} value={ p.prefix } component="button">{p.prefix}</DropdownItem>)) }
/> }
<TextInput
ref={ ref }
value={props.value}
isRequired
type="text"
id="url"
aria-describedby="url-helper"
name="url"
validated={ isUrlValid && isUrlAllowed && extraCheckResult === true ? "default" : "error"}
placeholder="e.g. 'http://example.com/api/hook'"
onChange={ value => {
if (props.setValid) {
props.setValid(isValidUrl(value) && prefixes.some(p => value.startsWith(p.prefix))
&& (!props.extraCheck || props.extraCheck(value) === true))
}
props.setValue(value)
} }
isDisabled={ props.isDisabled }
isReadOnly={ props.isReadOnly }
/>
</InputGroup>
</FormGroup>
)
}

export default HookUrlSelector;
6 changes: 4 additions & 2 deletions webapp/src/components/TestSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ type TestSelectProps = {
extraOptions?: SelectedTest[],
direction?: "up" | "down",
placeholderText?: string,
initialTestName?: string
initialTestName?: string,
isDisabled?: boolean,
}

export default ({ selection, onSelect, extraOptions, direction, placeholderText, initialTestName } : TestSelectProps) => {
export default ({ selection, onSelect, extraOptions, direction, placeholderText, initialTestName, isDisabled } : TestSelectProps) => {
const [open, setOpen] = useState(false)
const tests = useSelector(all)
const dispatch = useDispatch()
Expand Down Expand Up @@ -56,6 +57,7 @@ export default ({ selection, onSelect, extraOptions, direction, placeholderText,
}}
direction={direction}
placeholderText={placeholderText}
isDisabled={ isDisabled }
>{ [
...(extraOptions ? extraOptions.map((option, i) => <SelectOption key={i} value={option}/>) : []),
...(tests ? tests.map((test: Test, i: number) => <SelectOption
Expand Down
Loading

0 comments on commit f316f44

Please sign in to comment.