label-studio icon indicating copy to clipboard operation
label-studio copied to clipboard

feat: Feature/azure service principal

Open FrsECM opened this issue 1 year ago • 6 comments

PR fulfills these requirements

  • [x] Commit message(s) and PR title follows the format [fix|feat|ci|chore|doc]: TICKET-ID: Short description of change made ex. fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors
  • [ ] Tests for the changes have been added/updated (for bug fixes/features)
  • [ ] Docs have been added/updated (for bug fixes/features)
  • [ ] Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • [x] Self-reviewed and ran all changes on a local instance (for bug fixes/features)

Change has impacts in these area(s)

  • [ ] Product design
  • [x] Backend (Database)
  • [x] Backend (API)
  • [ ] Frontend

Describe the reason for change

For the moment, there is only an authentication through account key to Azure. The problem is that account key give a lot of rights on the storage that may not be necessary in order to perform readonly operations.

What does this fix?

Now it is possible to have an azure integration that is based on a service principal.

What is the new behavior?

We have to create an app registration in Azure :

We give this registration specific rights depending on what we want to do in labelstudio :

In the UI, we create a new storage integration :

Under the hood : image

What is the current behavior?

There is no impact on the previous behavior, it's just a new one.

What libraries were added/updated?

Azure Identity

Does this change affect performance?

Get a delegation key is quite slow, that's why there is :

  • 24h cache for delegation key
  • 15mn cache for sas token

There is a second minor change :

  • When a "thumbnail" tag is present in the datarow, it will be used in the grid view
  • When there is multiple layer in the image, the gridview will only render the first channel. (Today there is a buggy behavior).

Does this change affect security?

It should improve the security for people who just need specific rights on the container. For the moment, the "write" behavior have not been tested / developped on the container.

What alternative approaches were there?

We can use an enterprise application and SSO in order to be linked to a user right and act on behalf of him.

What feature flags were used to cover this change?

None

Does this PR introduce a breaking change?

(check only one)

  • [ ] Yes, and covered entirely by feature flag(s)
  • [ ] Yes, and covered partially by feature flag(s)
  • [ ] No
  • [ ] Not sure (briefly explain the situation below)

What level of testing was included in the change?

  • [ ] e2e
  • [ ] integration
  • [ ] unit

Which logical domain(s) does this change affect?

  • Authentication
  • UI

FrsECM avatar Apr 22 '24 15:04 FrsECM

Deploy request for label-studio-docs-new-theme pending review.

Visit the deploys page to approve it

Name Link
Latest commit a9b1e67deb07c18b621af1fab96a11799498bc86

netlify[bot] avatar Apr 22 '24 15:04 netlify[bot]

Deploy request for heartex-docs pending review.

Visit the deploys page to approve it

Name Link
Latest commit a9b1e67deb07c18b621af1fab96a11799498bc86

netlify[bot] avatar Apr 22 '24 15:04 netlify[bot]

I fixed the write access this morning. You can now create an export integration :

Without the right right in Azure, you can't use

But if you add : image

It will work and save the annotation :

FrsECM avatar Apr 23 '24 08:04 FrsECM

Could you also add pytest for this storage? There should be examples in tests/ folder for aws/gcs storages.

Can you help me on this part ? I'am not sure to understand how i'm suppose to create tests. The test i did locally were done by the UI and on my organisation private azure storage. I'm not able to replicate this in a public env.

FrsECM avatar Apr 23 '24 14:04 FrsECM

Sorry for the long delay, tests are located here: https://github.com/HumanSignal/label-studio/blob/develop/label_studio/tests/io_storages.tavern.yml#L1168

makseq avatar May 23 '24 11:05 makseq

Sorry for the long delay, tests are located here: https://github.com/HumanSignal/label-studio/blob/develop/label_studio/tests/io_storages.tavern.yml#L1168

Thanks,

I updated the get_import_export_storage_types test case to match the new syntax.

I saw that other things for gcp and s3, but in my case i have a strong validation that the connection is valid and can be consume, then i can't use dummy client_id/client_secret/tenant_id.

FrsECM avatar May 23 '24 14:05 FrsECM

/jira create

Workflow run Jira issue TRIAG-578 is created

sajarin avatar May 29 '24 16:05 sajarin

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

robot-ci-heartex avatar Jul 21 '24 01:07 robot-ci-heartex

This PR was closed because it has been stalled for 10 days with no activity.

robot-ci-heartex avatar Aug 01 '24 01:08 robot-ci-heartex