Skip to content

Commit

Permalink
Fix URL parsing and validation on Credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
baguse committed Aug 25, 2024
1 parent 917c44c commit 1aa789b
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 5 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,8 @@ Changelogs:
- 1.4.6 (13 August 2024)
* Fix module pre-register checking for directus 11
* Fix run flow webhook wrong method
- 1.4.7 (25 August 2024)
* Fix URL parsing and validation on Credentials. Now you can use the full URL with the protocol and it will be validated and parsed correctly

Contributing:
If you want to contribute kindly to create a PR and if you want to request a feature or report of a bug kindly create the Issue
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "directus-extension-flow-manager",
"description": "This is a custom module for managing Flow",
"icon": "extension",
"version": "1.4.6",
"version": "1.4.7",
"author": {
"name": "Bagus Andreanto",
"email": "[email protected]"
Expand Down
28 changes: 26 additions & 2 deletions src/flow-manager-module/components/credential-dialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const isPreviousIdPersisted = ref(false);
const isPullingFlows = ref(false);
const isPullingFlowError = ref(false);
const errors = ref<string[]>([]);
const selectedCredential = ref<ICredential>({
id: "",
name: "",
Expand Down Expand Up @@ -85,12 +87,25 @@ const flowManagerUtils = inject<{
}>("flowManagerUtils");
function saveCredential() {
let credentialUrlParsed = credentialUrl.value;
try {
const url = new URL(credentialUrlParsed);
if (!url.origin || url.origin === "null") {
errors.value = ["Invalid URL"];
return;
}
credentialUrlParsed = url.origin;

This comment has been minimized.

Copy link
@Lukas-Sachse

Lukas-Sachse Aug 27, 2024

Hi @baguse,
using only url.origin will break in our implementation.

We have our own web-app running on: app.test.com

And our directus runs on: app.test.com/api-service/ ← here the last / is important

image

This comment has been minimized.

Copy link
@baguse

baguse Aug 27, 2024

Author Owner

Ah I see, great catch. Thanks. Do you have a better approach? @Lukas-Sachse

This comment has been minimized.

Copy link
@Lukas-Sachse

Lukas-Sachse Aug 28, 2024

I was thinking about looking at standardized URL structures where you can extract the correct path. I was also a bit confused the first time, what URL I have to insert in the field in the credentials settings.

What do you think about always requesting and using the URL for the flow-manager in each environment?

Examples:
http://localhost:8055/admin/flow-manager
https://app.test.com/api-service/admin/flow-manager

With that we can search for the admin/flow-manager string in the URL and all that is written before is part of the directus path. And it also ensures that the flow-manager-extension was installed.

This comment has been minimized.

Copy link
@baguse

baguse Sep 9, 2024

Author Owner

Hi @Lukas-Sachse , the issue has been fixed on the v1.4.8

errors.value = [];
} catch (error) {
errors.value = ["Invalid URL"];
return;
}
if (isEdit.value) {
const newCredentials = [...credentials.value];
const credentialIndex = newCredentials.findIndex((credential) => credential.id === selectedCredential.value.id);
if (credentialIndex !== -1) {
(newCredentials[credentialIndex] as ICredential).name = credentialName.value;
(newCredentials[credentialIndex] as ICredential).url = credentialUrl.value;
(newCredentials[credentialIndex] as ICredential).url = credentialUrlParsed;
if (credentialStaticToken.value) {
(newCredentials[credentialIndex] as ICredential).staticToken = credentialStaticToken.value;
} else {
Expand All @@ -116,7 +131,7 @@ function saveCredential() {
{
id: generateRandomString(10),
name: credentialName.value,
url: credentialUrl.value,
url: credentialUrlParsed,
staticToken: credentialStaticToken.value,
},
];
Expand Down Expand Up @@ -276,6 +291,12 @@ async function proceedPull() {
v-tooltip.bottom="'Credential Static Token'"
class="input-form"
></v-input>
<v-error
v-for="(error, indexError) in errors"
class="mb-2"
:key="`errorIndex-${indexError}`"
:error="{ extensions: { code: 'Error' }, message: error }"
></v-error>
<v-button :disabled="!isValid" @click="saveCredential">
{{ isEdit ? "Save" : "Add" }}
</v-button>
Expand Down Expand Up @@ -323,6 +344,9 @@ async function proceedPull() {
margin-left: 10px;
}
.mb-2 {
margin-bottom: 10px;
}
.container > .v-card.card-extended {
--v-card-min-width: 1000px;
}
Expand Down

0 comments on commit 1aa789b

Please sign in to comment.