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

Auto Run UI #3031

Closed
wants to merge 8 commits into from
Closed

Conversation

LauchieHarvey
Copy link
Contributor

@LauchieHarvey LauchieHarvey commented Mar 2, 2024

Pull Request

Category

e.g. Bug, Module, Extension, Core Functionality, Documentation, Tests

Feature/Issue Description

Q: Please give a brief summary of your feature/fix
A: The user can now create, read, update and delete Auto Run rules from the Admin User Interface. This saves them from having build their own client or use a tool like cURL.

Q: Give a technical rundown of what you have changed (if applicable)
A:

UI Changes

I have added some ExtJS classes for the form input elements. The best way to test the UI changes is by running it, but I've attached a screenshot below too. It's not pretty. It might be worth experimenting with different UX designs, but my lack of experience with ExtJS slows me down when modifying the UI:
image

API Changes

I have also made some changes to the API. The changes to the API are why I have marked this as a draft, since I'm not sure if they're strictly necessary and I'm aware that changing the API can have unintended consequences for existing users.

  1. I added a PATCH endpoint to the API that allows the client to update an Auto Run Rule. The same thing could be achieved by deleting the old one and adding the modified rule. It's just a bit cleaner to let the API handle that logic in my opinion.
  2. I also modified the module list endpoint to return a list of objects instead of returning an object. This modification isn't necessary, but since the Wiki says that the keys in the returned object are meaningless I thought returning it in an array format would make it cleaner clients to use. To be more specific, the current endpoint is:
    image
    The format it returns the modules in has changed as depicted below:
// Current return format:
{
    "0": {
        "id": 1,
        "name": "Linksys WRT54G CSRF",
        "category": "Router"
    },
    "65": {
        "id": 69,
        "name": "Redirect Browser (Rickroll)",
        "category": "Browser"
    },
    ...
}
// New format returned by the endpoint in this PR, can be changed.
[{
        "id": 1,
        "name": "Linksys WRT54G CSRF",
        "category": "Router"
},
{
        "id": 69,
        "name": "Redirect Browser (Rickroll)",
        "category": "Browser"
},
    ...
}]

Test Cases

Q: Describe your test cases, what you have covered and if there are any use cases that still need addressing.
A: Honestly I haven't done much testing besides hooking a browser with a UI-added autorun rule and seeing that the Pretty Theft module ran successfully with command output.

Wiki Page

If you are adding a new feature that is not easily understood without context, please draft a section to be added to the Wiki below.

I will modify the Wiki to reflect API changes if and when we agree on them :) Do we think it would be worth adding Auto Run UI documentation to the Wiki too?

Additionally, I think there is a mistake in the Wiki describing the API since it said that the url /api/autorun/rule/list/all should list all of the rules, when the ruby API seems to expose that endpoint on /api/autorun/rules instead. My knowledge of ruby is comically minimal though, so I could well be incorrect.

@stephenakq
Copy link
Collaborator

Thank you for the PR. We will review soon

@stephenakq stephenakq had a problem deploying to Integrate Pull Request March 30, 2024 00:20 — with GitHub Actions Failure

it 'adheres to random bad passords and 1 correct auth rate limits' do
# create api structures with bad passwords and one good
passwds = (1..9).map { |i| "bad_password"} # incorrect password

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

This hardcoded value is
used as credentials
.
typeAhead: true,
listeners: {
select: async function(combo) {
const selectedModuleId = combo.getValue()

Check notice

Code scanning / CodeQL

Semicolon insertion Note

Avoid automated semicolon insertion (90% of all statements in
the enclosing function
have an explicit semicolon).
ruleContainer.removeAll();

for (let i = 0; i < rules.length; i++) {
ruleForm = new AutoRunRuleForm(

Check warning

Code scanning / CodeQL

Missing variable declaration Warning

Variable ruleForm is used like a local variable, but is missing a declaration.
* moveDown: moves the module down one spot in the Auto Run exection order.
*/
AutoRunModuleForm = function(moduleData, deleteFn, moveUp, moveDown, ruleId, index, moduleList) {
const moduleTextAreaId = `rule-${ruleId}-module-textarea-${index}`;

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused variable moduleTextAreaId.
*/
AutoRunModuleForm = function(moduleData, deleteFn, moveUp, moveDown, ruleId, index, moduleList) {
const moduleTextAreaId = `rule-${ruleId}-module-textarea-${index}`;
const chainModeComboId = `rule-${ruleId}-module-combo-${index}`;

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused variable chainModeComboId.
@stephenakq stephenakq had a problem deploying to Integrate Pull Request March 30, 2024 00:45 — with GitHub Actions Failure
@stephenakq stephenakq had a problem deploying to Integrate Pull Request April 23, 2024 18:39 — with GitHub Actions Failure
@LauchieHarvey LauchieHarvey marked this pull request as ready for review April 25, 2024 07:00
@LauchieHarvey
Copy link
Contributor Author

Thanks for the integration tests!

I forked the Wiki for this repo and made changes. I don't quite know how to include those changes in this PR though. Is it even possible to do that?

@stephenakq stephenakq had a problem deploying to Integrate Pull Request May 3, 2024 18:29 — with GitHub Actions Failure
Copy link
Contributor

Stale pull request message

Copy link
Contributor

github-actions bot commented Jun 6, 2024

Stale pull request message

@hastalamuerte
Copy link

@LauchieHarvey Thanks a lot for your work and implementation of Auto functional.
@stephenakq Maybe its time for new version release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants