dify icon indicating copy to clipboard operation
dify copied to clipboard

feat(tools): Preliminary OAuth Integration and Testing for API-based Tool Workflow

Open thompsonson opened this issue 1 year ago • 5 comments

Description

This pull request is integrates OAuth support within the API Based tool workflow.

Key updates and enhancements include:

  • Extending tool_entities.py to support OAuth, enabling tools to utilize OAuth authentication and authorization effectively.
  • Enhancing ApiBasedToolProviderController with OAuth-specific fields like client_id, client_secret, authorization_url, and token_url to the credentials schema, increasing flexibility for OAuth configurations.
  • Refining serialization and enum handling in common_entities.py and tool_entities.py to ensure accurate and internationally compatible OAuth support within tool credentials.
  • Implementing OAuth authentication type in the from_db method of ApiBasedToolProviderController, enriching the credentials schema to support a wider range of OAuth use cases.
  • Adding new ToolCredentialsOption entries in ToolProviderCredentials for extended authentication flexibility.
  • Updating .github/workflows/tool-tests.yaml and .gitignore for aligned CI processes and cleaner project structure.
  • Creating integration tests in test_api_tool.py and test_tool_provider_credentials.py to cover the new OAuth functionality and validate the serialization of tool provider credentials, ensuring existing authentication methods remain operational.

The integration tests are crucial in verifying the presence and correct types of expected keys in the credentials schema for each authentication type, ensuring the schema's accuracy and completeness.

Together, these changes significantly broaden the authentication capabilities of tool providers, enabling more secure and versatile integrations with API-based tools and services.

Fixes # (issue)

Type of Change

Please delete options that are not relevant.

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update, included: Dify Document

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [X] pytests included

Suggested Checklist:

  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] My changes generate no new warnings
  • [x] I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • [ ] optional I have made corresponding changes to the documentation
  • [ ] optional I have added tests that prove my fix is effective or that my feature works
  • [ ] optional New and existing unit tests pass locally with my changes

thompsonson avatar Feb 18 '24 15:02 thompsonson

Hello thanks for your contribution. 🥇 There is a test failed in CI. Could you fix it and I will run the workflow again.

crazywoola avatar Feb 19 '24 06:02 crazywoola

Thanks, it's a great platform so I'm keen to help improve it :)

I am having a problem running the tests locally.

image

I have "fixed" it locally by adding the api folder to the PYTHONPATH

export PYTHONPATH=$(pwd)/api:$PYTHONPATH

I then get errors as my API_KEYs are not valid, I assume this is as it is an integration test and actually going to the provider.

Have you come across the issue relating to PYTHONPATH before?

thompsonson avatar Feb 19 '24 07:02 thompsonson

I've committed a potential fix. It adds the api folder to the PYHTONPATH in the GitHub action.

I'm torn as I can re-create this locally, however I'm unsure as to why it is just me that has this problem :)

Please let me know what you think.

All the best, Matt

thompsonson avatar Feb 19 '24 09:02 thompsonson

Hi @thompsonson , could you leave style workflow files unchanged?

  1. style workflows are not the key purpose in this PR, please use small changes for different purposes
  2. do not change style.yaml to style-web.yaml, since SuperLinter is for general purposes on multiple formats rather just for lining ES/JS.
  3. as SuperLinter introduces much overhead time cost (2-3mins) for pulling image in pre-running steps, the lining Python job (costs 5-6 sec) are designed as the preposition before the SuperLinter job

Thanks.

bowenliang123 avatar Feb 19 '24 12:02 bowenliang123

Hi @thompsonson , could you leave style workflow files unchanged?

  1. style workflows are not the key purpose in this PR, please use small changes for different purposes
  2. do not change style.yaml to style-web.yaml, since SuperLinter is for general purposes on multiple formats rather just for lining ES/JS.
  3. as SuperLinter introduces much overhead time cost (2-3mins) for pulling image in pre-running steps, the lining Python job (costs 5-6 sec) are designed as the preposition before the SuperLinter job

Thanks.

I've reverted the styme changes. Is that OK?

I've still to fully learn about Dify's way of doing things, getting there. :) I'll research SuperLinter as I'd thought it was just JS/TS. 👍

thompsonson avatar Feb 19 '24 14:02 thompsonson

Hello can you reopen this from the latest branch. It seems lots of conflicts here. And seems it used a outdated github action as well.

crazywoola avatar Apr 18 '24 10:04 crazywoola