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

Issue 1426 - Source validation on the claim form #1428

Open
wants to merge 9 commits into
base: stage
Choose a base branch
from
3 changes: 2 additions & 1 deletion public/locales/en/sources.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"sourceCardButton": "Access the source",
"sourceCreateCTAButton": "Create source",
"sourcesCreateSuccess": "Source created successfully",
"sourcesCreateError": "Error while creating the source"
"sourcesCreateError": "Error while creating the source",
"sourceInvalid": "The font is no longer available"
}
3 changes: 2 additions & 1 deletion public/locales/pt/sources.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"sourceCardButton": "Acesse a fonte",
"sourceCreateCTAButton": "Incluir fonte",
"sourcesCreateSuccess": "Fonte criado com sucesso",
"sourcesCreateError": "Erro ao criar a fonte"
"sourcesCreateError": "Erro ao criar a fonte",
"sourceInvalid": "A fonte não está mais disponível"
}
17 changes: 17 additions & 0 deletions server/source/source.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,23 @@
);
}

@ApiTags("source")
@Get("/check-source")
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue(blocking): now that I'm thinking, this is a DDOS vector and we should not continue. We can consider a few options like putting this behind RBAC for logged-in users.

But why aren't we just requesting from the client-side? Why we need to create a new endpoint for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The endpoint was needed because of CORS restrictions, many sources block client-side requests. After, I can look with @lucaslobatob at some ways to make it more secure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still not satisfied with the current solution, there are still too many vulnerabilities. Let's sync and create an epic documentation before proceeding.

async checkSpurce(@Query() query) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: typo

try {
const result = await fetch(query.source, {
method: "GET",
});
Fixed Show fixed Hide fixed
if (result.ok) {
return { status: 200, message: "Link válido"};
} else {
return { status: 404, message: "Link inválido"};
}
} catch (error) {
return { status: 500, message: "Erro interno ao verificar o link"};
}
}

@IsPublic()
@ApiTags("source")
@Get("api/source")
Expand Down
11 changes: 11 additions & 0 deletions src/api/sourceApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,22 @@ const getById = (id, t, params = {}) => {
});
};

const checkSource = (source) => {
return axios.get('/check-source', {params: {source}})
.then((response) => {
return response.data;
})
.catch(() => {
message.error("error");
});
}

const SourceApi = {
getByTargetId,
createSource,
get,
getById,
checkSource,
};

export default SourceApi;
27 changes: 25 additions & 2 deletions src/components/Claim/CreateClaim/BaseClaimForm.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useState } from "react";
import { Checkbox, Form, Row } from "antd";
import { Checkbox, Form, Row, message } from "antd";
import moment from "moment";
import { useTranslation } from "next-i18next";
import { useRouter } from "next/router";
Expand All @@ -10,6 +10,7 @@ import Input from "../../AletheiaInput";
import Button, { ButtonType } from "../../Button";
import DatePickerInput from "../../Form/DatePickerInput";
import SourceInput from "../../Source/SourceInput";
import SourceApi from "../../../api/sourceApi";

interface BaseClaimFormProps {
content?: React.ReactNode;
Expand Down Expand Up @@ -46,7 +47,29 @@ const BaseClaimForm = ({
setDisableSubmit(!hasRecaptcha);
};

const onFinish = () => {
const checkLink = async (url) => {

try {
const request = await SourceApi.checkSource(url);
if (request?.status === 200) {
return true;
} else {
message.error(t("sources:sourceInvalid"));
return false;
}
} catch (error) {
message.error(t("sources:sourceInvalid"));
return false;
}
}

const onFinish = async () => {

const validSource = await Promise.all(
sources.map((source) => checkLink(source)));

if (validSource.includes(false)) return;

const values = {
title,
date,
Expand Down
37 changes: 33 additions & 4 deletions src/components/Source/ClaimSourceListItem.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
import { Col, List } from "antd";

import { Col, List, message } from "antd";
import React from "react";
import { useState } from "react";
import SourceApi from "../../api/sourceApi";
import { useTranslation } from "next-i18next";

const ClaimSourceListItem = ({ source, index }) => {
const { href } = source;
const [linkValid, setLinkValid] = useState(null);
const { t } = useTranslation();

const checkLink = async (url) => {

try {
const request = await SourceApi.checkSource(url);

if (request?.status === 200) {
setLinkValid(true);
window.open(url, "_blank", "noopener noreferrer");
} else {
setLinkValid(false);
message.error(t("sources:sourceInvalid"));
}
} catch (error) {
setLinkValid(false);
message.error(t("sources:sourceInvalid"));
}
};

return (
<Col className="source">
Expand All @@ -12,15 +34,22 @@ const ClaimSourceListItem = ({ source, index }) => {
<span style={{ marginRight: 4 }}>
{source?.props?.sup || source?.sup || index}.
</span>
<a href={href} target="_blank" rel="noopener noreferrer">
<a href={"#"}
onClick={(e) => {
e.preventDefault();
checkLink(href);
}}>
{href}
</a>
</List.Item>
) : (
<List.Item id={source}>
{/* TODO: Remove this ternary when source migration is done */}
{index}.{" "}
<a href={source} target="_blank" rel="noopener noreferrer">
<a href={"#"} onClick={(e) => {
e.preventDefault();
checkLink(source);
}}>
{source}
</a>
</List.Item>
Expand Down
Loading