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

Deprecate and remove the exportPublic + ACL-based approach to exported object expiry #2054

Closed
lmsurpre opened this issue Mar 11, 2021 · 4 comments
Assignees
Labels
bulk-data configuration deprecation P2 Priority 2 - Should Have removal this change involves removal of a component, class, method, etc security

Comments

@lmsurpre
Copy link
Member

Is your feature request related to a problem? Please describe.
When the export is configured to create public objects (fhirServer/bulkdata/storageProviders/(source)/exportPublic = true), we attempt to set an ACL policy for objects to make them temporarily public. I believe that is currently hard-coded to 2 hours.

Unfortunately, neither minio nor IBM COS actually honor this setting:

Please note that IBM COS does not support expiration time for each single COS object, so please configure retention policy (e.g, 1 day) for the bucket if IBM COS is used.

Although its documented, this seems like it could end badly (exports staying public too long and hacked/stolen because of it).

In #882 we added support for using presigned URLs for clients to fetch the resources and that seems like a better approach.

Describe the solution you'd like
Deprecate and eventually remove support for fhirServer/bulkdata/storageProviders/(source)/exportPublic.
Guide users toward using HMAC auth and presigned URLs instead.

Describe alternatives you've considered
Continue to support setting ACLs.

Acceptance Criteria
Documentation is clear that HMAC auth and presigned URLs is the preferred approach.

Additional context
Originally discussed at https://github.com/IBM/FHIR/pull/2052/files#r592359447

@lmsurpre lmsurpre changed the title Deprecate the isPublic + ACL-based approach to exported object expiry Deprecate the exportPublic + ACL-based approach to exported object expiry Mar 11, 2021
@lmsurpre
Copy link
Member Author

two locations: config and bulkdata util
mostly just testing

@lmsurpre lmsurpre added P2 Priority 2 - Should Have deprecation labels May 17, 2021
@prb112 prb112 self-assigned this May 18, 2021
prb112 added a commit that referenced this issue May 18, 2021
expiry #2054

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 added this to the Sprint 2021-07 milestone May 18, 2021
@lmsurpre lmsurpre changed the title Deprecate the exportPublic + ACL-based approach to exported object expiry Deprecate and remove the exportPublic + ACL-based approach to exported object expiry May 21, 2021
@lmsurpre
Copy link
Member Author

The deprecation was done in 4.8.1.
The removal will be done via #2380 for 4.9.0.

@lmsurpre lmsurpre added the removal this change involves removal of a component, class, method, etc label Jun 7, 2021
@lmsurpre
Copy link
Member Author

I reviewed the results of #2380 and I think we should go even further...I opened #2543 with a couple additional removals including a proposal to remove the legacy bulkdata config that has been deprecated since 4.6.0.

@lmsurpre
Copy link
Member Author

#2543 is merged and I'm calling this one done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bulk-data configuration deprecation P2 Priority 2 - Should Have removal this change involves removal of a component, class, method, etc security
Projects
None yet
Development

No branches or pull requests

2 participants