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

Backend: Add API endpoint for getting files by sha1 hash #259

Merged
merged 2 commits into from
Feb 26, 2020

Conversation

thielepaul
Copy link
Contributor

This pull request adds a new endpoint for getting file information by their hash.

The PhotoPrism mobile app will use this endpoint

@lastzero
Copy link
Member

This API doesn't exist intentionally so that you can't get information about a photo library without additional information / auth. GET requests don't support authentication.

@thielepaul
Copy link
Contributor Author

Im not sure if I understand your concerns correctly. Of course, this API endpoint should only be accessible by authenticated users.
However, I do not understand why GET requests don't support authentication. Anyway, I would be open to change the request type if that helps.

@lastzero
Copy link
Member

lastzero commented Feb 26, 2020

Images are typically loaded by a standard component like the <img> tag. Or the user just takes the link and opens the file manually. In this case, there won't be any additional http headers added to the request. From what I know, auth is not implemented in the mobile app currently? So it wouldn't make a difference as the auth check will always pass.

@thielepaul
Copy link
Contributor Author

thielepaul commented Feb 26, 2020

Authentication is implemented in the mobile app since today.
However, I still do not understand, what is the problem with this merge request?

@lastzero
Copy link
Member

At least in our Web app, we can not change request headers for images since browsers load them without AJAX (regular GET request, no JS involved, so no extra headers possible).

If you can't check authentication for any reason (like in our Web app), the API would let anyone know if you own a certain photo as you can simply ask for an image with a given SHA1 hash (404 if not). Ideally, you use the same API for photos we use for our Web app as this is pretty much secure, even without extra auth and we don't have to maintain two APIs.

I agree that GET requests with auth token in the http header would be secure, but that's a special case and only possible if you don't use HTML / JS. But also it means you can't give this link to a user as it wouldn't work. We need to be aware if the implications here.

There are many reasons you don't want to let others know what images you have. It might be pr0n, it might be politically sensitive photos etc...

@thielepaul
Copy link
Contributor Author

thielepaul commented Feb 26, 2020

I still have the feeling that we are talking about different things.
The new endpoint introduced in this merge request is very similar to the // GET /api/v1/photos/:uuid endpoint only that not the photo UUID is the parameter but the filehash.
Both endpoints are only available to authenticated users, both endpoints to not return an image but information from the database.
There is no additional information accessible through the new endpoint that is not available through existing endpoints, it is only accessible in a more convenient way for the mobile app.

@lastzero
Copy link
Member

I see, sorry. Thought you return the binary image. Any reason the uuid does not work for you?

@lastzero lastzero merged commit 1d89858 into photoprism:develop Feb 26, 2020
@thielepaul
Copy link
Contributor Author

If the app is reinstalled, I do not know which photos have already been uploaded (and their UUIDs) but I know the sha1 hashes of the photos on the phone.
This endpoint is a convenient way of getting the UUID from the sha1 hash.

@lastzero
Copy link
Member

Merged this, guess it has something to do with how you upload photos. I'm a bit distracted right now, sorry.

@thielepaul
Copy link
Contributor Author

@lastzero If you got a minute, it would be nice if you could merge this into the master branch, so that I can merge the feature into photoprism-mobile (thielepaul/photoprism-mobile#55)

@lastzero
Copy link
Member

lastzero commented Mar 9, 2020

Done!

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

Successfully merging this pull request may close these issues.

None yet

2 participants