search

RelatedTitle / user-account-system

This user account system allows for the registration of users using their email and password + OAuth providers. It has full email verification and password reset functionality. It uses PostgreSQL, Express, Passport, and JWT as user tokens.
MIT License
2 stars 0 forks source link

Add support for Cloudflare images. #12

Open RelatedTitle opened 3 years ago

RelatedTitle commented 3 years ago

Add support for storing user avatars on Cloudflare images.

AlexVCS commented 3 years ago

I'd like to work on this issue, please assign it to me.

RelatedTitle commented 3 years ago

Any updates?

AlexVCS commented 3 years ago

I'm working on it I'll give you a more thorough update tomorrow.

AlexVCS commented 3 years ago

Ok I'm trying to setup my config and still have a way to go with that. I'm sorry I'll keep working on it but I know I don't have much time.

RelatedTitle commented 3 years ago

Please let me know if you have any issues with the config or if the docs aren't explained well, it's my first time making an open-source project so I'm not sure if it's explained correctly. :)

AlexVCS commented 3 years ago

For sure, there are parts I'm not sure I need to fill in or not. Also, there are parts I need to fill in that aren't listed in the Config page. For example, the config.email.smtp on line 88 isn't listed on that page. I am new to this sort of project and am looking to get the project running locally to move forward with the contribution.

RelatedTitle commented 3 years ago

You're not supposed to fill anything in on config.email.smtp, only config.email.smtp.hostname, config.email.smtp.port, config.email.smtp.secure, etc. Same goes for config.user.avatar and others. They're just there as "placeholders" so to speak. I should probably make this clearer in the docs. Everything after trustscore also isn't being actively used right now.

RelatedTitle commented 3 years ago

You also don't need to fill in everything, like the OAuth or S3 keys for example. Those are only required if you are planning to use OAuth or S3.

AlexVCS commented 3 years ago

Right now when I run npm run dev it says TypeError: Cannot set property 'smtp' of undefined so that's why I figured I needed to fill that in.

RelatedTitle commented 3 years ago

I'm sorry about that. It looks like I forgot to add config.email = {}; on line 6 in the config example file. This has been fixed in the newest commit (along with an issue where not providing OAuth keys would result in an error).

AlexVCS commented 3 years ago

No worries. I updated my config with that on line 6. I'm getting this error now (looking into it, but in case you know a quick solution): `/Users/alex/Desktop/Projects/user-account-system/node_modules/sequelize/lib/sequelize.js:187 options.dialect = urlParts.protocol.replace(/:$/, '');

TypeError: Cannot read property 'replace' of null at new Sequelize (/Users/alex/Desktop/Projects/user-account-system/node_modules/sequelize/lib/sequelize.js:187:43)`

RelatedTitle commented 3 years ago

That means you most likely haven't provided a database connection string. (Looks like this: postgres://username:password@hostname:port/database_name]). On the config.db.connection_string config key. You'll need to download PostgreSQL, or you can use a service like ElephantSQL that has a free tier.

RelatedTitle commented 3 years ago

Once you get it running, you'll probably want to look in the util/avatar.js file, as that's where everything related to processing and storing avatars is. You may want to look at one of the existing functions like store_s3 or store_locally as guidance and create a new function (something like store_cloudflare). You'll also have to add config keys like with S3 for the API tokens and a new case in the store_avatar function.

AlexVCS commented 3 years ago

Ok, thanks I got all of that added. The error I have now seems to be related to what you mentioned earlier about OAuth. It says OAuth2Strategy requires a clientID option. How about that error? I'm going to be working on this more tomorrow.

RelatedTitle commented 3 years ago

This has been fixed in the latest commit.

AlexVCS commented 3 years ago

For the JWT section under User in config, can we use the example values you provided? I tried them and then got this error `events.js:292 throw er; // Unhandled 'error' event ^

Error: listen EACCES: permission denied 0.0.0.0:80`

RelatedTitle commented 3 years ago

Yes, you can use the example values (not for production though). That error seems to be unrelated to the JWTs. Could you try changing the port number in line 120 in server.js to something higher than 1024.

AlexVCS commented 3 years ago

Ok I don't get any errors now. I'm a little unsure of how to check on the running of the app. I opened Postman to try and access it after running npm run dev. What's the best way to check that it's up and running?

RelatedTitle commented 3 years ago

You can try an endpoint like /auth/register or /auth/login. You don't really need to fill in anything if you don't want to create a user or log in, it will respond with an error but you'll know it's running.

AlexVCS commented 3 years ago

Ok but does the localhost:Number/auth/register need to be set to what I put in server.js on line 120?

RelatedTitle commented 3 years ago

Yes, that's the port. So if you set something like 3000 it would be localhost:3000/auth/register

AlexVCS commented 3 years ago

I'm working on the function and signed up for a Cloudflare account. I just wanted to clarify that I need to have an account and pay in order to get this working right?

RelatedTitle commented 3 years ago

Yes, I believe there is a $5 minimum charge, I don't think there is a free tier. This is one of the reasons that I opened this issue, I was looking for someone that already used Cloudflare images that could test it.

AlexVCS commented 3 years ago

Ok I understand. My apologies for not being aware of this before. If I signup for the minimum could I complete the issue and then just cancel the service so it won't continue to charge me?

RelatedTitle commented 3 years ago

I believe so. It's $5 per 100k images stored a month (which is prepaid) and $1 per 100k images served (postpaid). It's ok if you do not wish to complete the issue due to this, just please let me know.

AlexVCS commented 3 years ago

Ok I went ahead and spent the $5. I still want to complete this and will continue to ask questions as I need to. Thanks

AlexVCS commented 3 years ago

Should I be following the instructions here: https://developers.cloudflare.com/images/cloudflare-images/upload-images/direct-creator-upload

or the make image private?

RelatedTitle commented 3 years ago

I believe those instructions are for the user uploading the image directly to Cloudflare from the frontend, that is not what we want. The images should be uploaded via the /user/change_avatar endpoint, processed, and uploaded to Cloudflare on the backend. The images should be public by default.

AlexVCS commented 3 years ago

I'm not finding many resources on how to implement this with Cloudflare. Are you aware of any that you could share? Thanks

AlexVCS commented 3 years ago

I've looked into what Cloudflare offers and don't see anything about uploading images from a backend. Do you have links to more info on what they offer for this please?

RelatedTitle commented 3 years ago

https://api.cloudflare.com/#cloudflare-images-upload-an-image-using-a-single-http-request

AlexVCS commented 3 years ago

Hey there, sorry for the delay. I have a draft of the function and added a case to store_avatar. I'm not certain it works yet, but would you like to see the code?

RelatedTitle commented 3 years ago

Sure, I'd like to see it. Have you tried making a user and uploading the avatar?

AlexVCS commented 3 years ago

Ok I think it's a little premature for a PR but here's what I pushed on my forked repo: https://github.com/AlexVCS/user-account-system/tree/cloudflare_image_support I haven't tried to do that, I definitely need to though.

RelatedTitle commented 3 years ago

Sorry for the delay. I haven't tested it but it looks good. The only thing is you forgot to include a config-example.js file with the new config keys and you committed the port change in server.js from 80 to 3000 I'd also change "cloudFlare" to just "cloudflare" for consistency but that's more of an aesthetic thing.

AlexVCS commented 2 years ago

No worries I'm sorry for my delay! I renamed config-example.js to config.js as the setup docs say. The config.js is in the git ignore. Do you want just the part about cloudflare to be added back to a config-example file?

RelatedTitle commented 2 years ago

The config.js file should not be included, as it contains sensitive information. You should include the default config-example.js file (you can find it here) with the new keys that you added.

AlexVCS commented 2 years ago

Ok I understand, but one of the new things I added is the secret access key for Cloudflare. Should I be committing that publicly?

RelatedTitle commented 2 years ago

No, not your actual secret access key. What I meant by "key" was the config value name (like config.user.avatar.s3.access_key). You should just add the name of the config value but leave the string blank.

Example:

config.user.avatar.cloudflare = {};

config.user.avatar.cloudflare.secret_access_key = "";
// ...
AlexVCS commented 2 years ago

Ok great, thanks for the info. I added the config-example and updated it to include those new fields.