nopSolutions / nopCommerce

ASP.NET Core eCommerce software. nopCommerce is a free and open-source shopping cart.
https://www.nopcommerce.com
Other
9.19k stars 5.28k forks source link

UPS API Security Migrating to OAuth #6712

Closed cklenk closed 9 months ago

cklenk commented 1 year ago

UPS API is switching from access keys to OAuth starting June 3, 2024. Will the UPS shipping plugin have to be updated to accommodate this? Here are some notes about the changes.

Thanks Cameron

https://developer.ups.com/oauth-developer-guide?loc=en_US&sp_rid=NTA5MzQ1OTE2NjEyS0&sp_mid=72989914 https://developer.ups.com/api/reference#tag/OAuthAuthCode_other

danFbach commented 1 year ago

To my knowledge, it is also no longer possible to request new access keys from UPS - when i built a store a few months back, i had to pull the keys from the database of their previous store because UPS would not allow me to create new ones. So, in my opinion, the plugin is already in a broken state.

cklenk commented 1 year ago

To my knowledge, it is also no longer possible to request new access keys from UPS - when i built a store a few months back, i had to pull the keys from the database of their previous store because UPS would not allow me to create new ones. So, in my opinion, the plugin is already in a broken state.

According to the migration guide it says Beginning June 5, 2023 UPS will no longer distribute access keys and all API transactions after June 3, 2024 with UPS will require clients to implement the OAuth security model by sending a bearer token with every API request.

danFbach commented 1 year ago

@cklenk fair enough. maybe they just buried it as a way of discouraging their use. Their management interface is quite difficult to navigate in general. Either way, this will be a problem in the near future.

AndreiMaz commented 1 year ago

Also see https://www.nopcommerce.com/en/boards/topic/97068/ups-api-will-require-oauth-2-after-june-3-2024

skoshelev commented 1 year ago

Hi @cklenk. UPS not only switched to a new authorization method, they also changed the API along with it. We have completed the transition of the codebase to new calls, but, unfortunately, at the moment we are not able to test the functionality. If you have the desire and the technical ability to help us with testing, you can build a new plugin from this brach. We will be grateful for any help.

cklenk commented 1 year ago

Hi @cklenk. UPS not only switched to a new authorization method, they also changed the API along with it. We have completed the transition of the codebase to new calls, but, unfortunately, at the moment we are not able to test the functionality. If you have the desire and the technical ability to help us with testing, you can build a new plugin from this brach. We will be grateful for any help.

Awesome! I can see what I can do :) Thanks!!

danFbach commented 1 year ago

@skoshelev I'll give the API a go on our store and let you know how it goes.

danFbach commented 1 year ago

@skoshelev Seems to work fine so far. The new locale resources did not populate though because i just updated it, i did not uninstall and reinstall the plugin. Perhaps add the new resources to UpdateAsync function?? I'll let you know if anything else comes up.

nealculiner commented 1 year ago

@AndreiMaz it appears in reviewing the code you're keeping the same plugging but gutting all of the code to replace with the new OAUTH system. It may be prudent to leave the current plugin as is so people will not have their existing implementation overwritten as we really don't know when UPS is going to really take down the legacy system, they may extend it. Also people may want to hold on to the old integration while testing the new, etc. I suggest the new OAUTH is a new plugin standalone from the existing UPS plugin. Maybe name it something with OAUTH in it as we probably can't change the name of the prior plugin to *.Legacy.UPS. The alternate option would be a radio button switch in the plugin to use legacy provider or new provider. Food for thought...

danFbach commented 1 year ago

@nealculiner @AndreiMaz My two cents... Since you can no longer request the old version of API key's that the old UPS plugin version would use, i would personally say it is effectively already phased out for any new installations. My suggestions would be to detach the UPS plugin from the standard nop source code, do not include it in new downloads, and start a standalone git repo for it - as essentially all of the other shipping plugins are. Then users could maintain their lagacy installation of the plugin for as long as they're comfortable.

danFbach commented 1 year ago

Also, I've been running the updated plugin without issue. No bugs to report. I would encourage others (@cklenk) to try it too as a sample size of 1 is not much of test pool.

nealculiner commented 1 year ago

@danFbach @AndreiMaz if ready for beta maybe make it available as a plugin to be installed for testing. But you're going to have to address overwriting/updating the existing plugin. I'm still in development and not yet in production so I can certainly move to it and test but this would be a concern otherwise.

cklenk commented 11 months ago

Also, I've been running the updated plugin without issue. No bugs to report. I would encourage others (@cklenk) to try it too as a sample size of 1 is not much of test pool.

So sorry we haven't gotten around to testing this yet. But we can try to put on our schedule! Thanks.

lakipolitis commented 11 months ago

@skoshelev, I have a client looking to implement this updated version of the UPS API. But they're using SimpleRate. I see that there's references in the new plugin to the SimpleRate functionality, but there's no method to use the SimpleRate flag when getting Shipping Rates. Am I missing something, or was that not implemented? If it wasn't, I would be happy to participate and contribute that code.

I'm going to build my modifications to the current plugin with the mindset that SimpleRate would apply differently based off of the Packing Type:

I've been a long time nopCommerce user/developer, and have never contributed to the project. I'd be happy to finally give back to a fantastic open-source product I've used for years.

skoshelev commented 9 months ago

Hi @lakipolitis. As I understand this is a new proposal from UPS and we don't really have an implementation for it, we would appreciate your input if you can find the time to extend the functionality of our plugin

skoshelev commented 9 months ago

Closed #6712

cklenk commented 8 months ago

Is the plan still to release this with 4.7? And will that be ready in time for the UPS change?

nealculiner commented 7 months ago

I'm testing out the plugin from the develop branch here but I'm getting auth errors from UPS. I signed up for the OAUTH and have the client id / secret, I'm not sure what is supposed to go into the callback URL on the app config at UPS, if anything. Can anyone share steps to get this working?

danFbach commented 7 months ago

I've been using the OAUTH version since i posted September. no errors. I remember it took me a a few tries to get it set up correctly too, but that was several months ago now, so i'm not sure what i had to do. @nealculiner Can you share any error logs?

nealculiner commented 7 months ago

I've been using the OAUTH version since i posted September. no errors. I remember it took me a a few tries to get it set up correctly too, but that was several months ago now, so i'm not sure what i had to do. @nealculiner Can you share any error logs?

Status: 401 Response: {"response":{"errors":[{"code":"250002","message":"Invalid Authentication Information."}]}}

danFbach commented 7 months ago

My configuration (on UPS api website) has no callback url. Make sure that you enable the correct products, I have Rating, Tracking and (by default) Authorization. My nop configurations looks like this. image

danFbach commented 7 months ago

Is your key status approved on UPS? image

danFbach commented 7 months ago

@skoshelev If i remember correctly, setting up the UPS developer account and UPS api "app" was a struggle, as their instructions are too generic and confusing for the many users, it even took me awhile to wade through it all to get it right. Perhaps we should put together a guide to support this integration before it is officially released?

nealculiner commented 7 months ago

Yes @danFbach I'm approved. I didn't add any "products" other than OAUTH on the right side. I just now added a few more of those like shipping, tracking, etc. and it's all instantly approved. What "products" do you have enabled?

And yes this setup is not straight forward and nopCommerce hopefully is aware or it's going to generate a lot of noise.

danFbach commented 7 months ago

This is all that i have enabled. image

nealculiner commented 7 months ago

Thanks @danFbach RATING is the one I missed and must be required. It works now.

danFbach commented 7 months ago

@nealculiner Good to hear... I believe that in an ideal integration, the plugin itself defines all of its own requirements and the configuration page would just require the user to enter their UPS credentials. Then the api keys would be automatically generated and retrieved, and the website admin would not have to set any of this up.

nealculiner commented 7 months ago

@nealculiner Good to hear... I believe that in an ideal integration, the plugin itself defines all of its own requirements and the configuration page would just require the user to enter their UPS credentials. Then the api keys would be automatically generated and retrieved, and the website admin would not have to set any of this up.

I agree, it should be configured at the admin plugin config via API with checkboxes at the plugin config as to what to enable or not once you have your client id/secret.

sangeet-shah commented 5 months ago

@AndreiMaz i have taken UPS plugin from latest development repository and test but i am getting Alert json parsing error in log list page.

There reason is response parsing issue.

image

To resolve this issue i have removed alert property. 1