Merging Two tasks fails for lack of permission ( it seems )

[patt0]

Error messages ( twice as I was merging two activities )

The merged entity has been added, but the other two are still here.

[Scarygami]

Is the new activity actually there after a page refresh? Or was it only added in the front-end data but never appeared in the backend? Delete and insert actions should have the same permission check, so it's weird that one works while the other doesn't...

[Scarygami]

Btw, this shouldn't really be affected by your changes, since you didn't touch the relevant parts (I think), so you might have encountered a bug we haven't come across yet.

[patt0]

Yes I assumed, but I did some changes to utils.py and changed secret.json too hold two keys instead of 1, just to be sure.

I am going to delete the database, and restore from my backup. I will also push to the staging backend (firefly) Then we can test this insert on the new staging front end that points to firefly.

Patrick Martinent

On Mon, Oct 27, 2014 at 8:11 PM, Gerwin Sturm notifications@github.com wrote:

Btw, this shouldn't really be affected by your changes, since you didn't touch the relevant parts (I think), so you might have encountered a bug we haven't come across yet.

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/149#issuecomment-60603309.

[SmokyBob]

Looking into it on the Staging environment Update 1: found some bugs on the merge (ex. impact not correctly summed up, other fields not merged correctly) but NOT able to reproduce this one on the staging environment. Going to dinner and looking deeply in the issue, maybe is related to the 2 activities (ex. partial metadata, some old fields, ...) and I'm not able to reproduce the issue with activity created from scratch

[SmokyBob]

Now that I look at the screenshot better... seems like the merged activity is created but the "old' activities failed to be removed...

Edit1: Can you try again and see if there is a message in the Dev Tool console like gdeTrackingAPI.activity_record.delete({id:... ?

Edit2: I might have find out why... The account you log in is the one associated to 'patrick.martinent AT gmail.com' ?

Edit3: Checked the logs... The insert and the 2 delete operations completed correctly without returning any message... (approx 14:56 CET)

[SmokyBob]

I was able to reproduce the error on the staging environment with those 2 particular activities (tricked the system thinking I'm @patt0 ).

From the staging backend logs :

• Insert OK (22:49:55.027 CET)
• Delete AR 1 - KO - Not Auth (22:49:58.806 CET)
• Delete AR 2 - KO - Not Auth (22:49:59.944 CET)

I tried to track where the message Only GDEs and admins may enter or change data. is thrown and fount it on web_endpoints.py line 72.

Then check_auth is called and subsequently get_current_account responsible to log Authenticated user: mauro.solcia@gmail.com

In my understanding that should be ok and delete should work.

Tried to merge 2 of my own activities "auto created" by the nightly task (logs from 21:59:22.735 to 21:59:26.384) and everything worked as expected.

The 2 merges were done trying to provide activity_record.delete with the api_key to avoid authentication by email and possible problems like #133 (see latest commit in my branch issue149 for the code used during the tests).

[Scarygami]

Is it possible that there is some stray symbol in the email list? When I try to retrieve Patricks account via email address Account.query(email='patrick.martinent@gmail.com') I get no results. But it works for other email addresses... Since the backend can't retrieve a valid user it throws the error.

You can try this at https://console.developers.google.com/project/omega-keep-406/datastore/query

Patrick would have had those issues independent of the changes we made.

@SmokyBob While you can simulate @patt0 in the frontend, the backend will still check against your access token, knows that you are mauro.solcia@gmail.com and won't let you edit/delete/update records of anyone else, which is why this check is there, so it is expected that this doesn't work for you, even if you can simulate the requests from the frontend.

Note: Since the admin_api_key is now "leaked" publicly we will have to change it once the authentication issues with multiple emails have been fixed, since it allows anyone without any extra authentication to change/delete/insert data into the API. Once the authentication issues have been fixed everything should work via normal OAuth authentication and it should then be removed from the frontend.

[Scarygami]

Update: looking at the query result in the console indeed reveals an extra space in the email for Patrick, which causes the authentication error. Since you supply the api_key for inserts (as I mentioned above this should be removed later) the error doesn't occur then, and only comes up at the subsequent delete steps. Fixing the email in the list and updating the Account list should fix this issue.

[SmokyBob]

@Scarygami thx, the only thing I forgot to check is the GDE Masterlist where the email was stored with an extra space; pushing the updated value as we speak and adding a trim before pushing from the masterlist to the backend.

My bad for putting the API key publicly inside the code (came in mind only yesterday that I could have used a non tracked file like is done in the backend); The api_key is used as a temporary fix to #133, once that is fixed, we can drop the current key and generate a new one for the Sheets (Masterlist AT,PG,AG)

[patt0]

Nice catch !!!

I have updated my account using the DataStore viewer, reset the Writes on the datastore and testing that it works.

The total impact graph does not update itself after you update View / Impact data. I also think we might want to display Log(Impact).

Patrick Martinent

On Tue, Oct 28, 2014 at 2:52 PM, Gerwin Sturm notifications@github.com wrote:

Update: looking at the query result in the console indeed reveals an extra space in the email for Patrick, which causes the authentication error. Since you supply the api_key for inserts (as I mentioned above this should be removed later) the error doesn't occur then, and only comes up at the subsequent delete steps. Fixing the email in the list and updating the Account list should fix this issue.

[image: extra_space] https://cloud.githubusercontent.com/assets/809528/4805965/a8c70112-5e83-11e4-9590-19b1f91b4386.png

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/149#issuecomment-60728179.

[Scarygami]

@SmokyBob probably for now best to also include the apikey in the delete requests (as in your tests) since #133 would affect those as well. Also since the code is client-side it would be exposed when you look at the source of the page either way. We are in a stage where this isn't really critical, but I recently held a talk about just the topic of secrets in open source, so now I'm extra cautious about this :)

[SmokyBob]

Quick fix to the GDE Masterlist applied. @Scarygami Yep, going to merge the use of api_key on delete later today

[SmokyBob]

Deployed #153 so we should be ok with merges even in #133 cases.

@patt0 yes... the arrays used by the graph and table are updated but the graphs don't seems to catch this change... I'll open an issue and look into it and the chart API. Can you clarify what do you mean by "display Log(Impact)"?

[SmokyBob]

merge should be fixed, deployed code to keep the graph and totals table updated after activity edit. Can we close this issue or something is still missing?

[patt0]

We are ok I believe

On Sat Nov 01 2014 at 6:36:43 PM Mauro Solcia notifications@github.com wrote:

merge should be fixed, deployed code to keep the graph and totals table updated after activity edit. Can we close this issue or something is still missing?

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/149#issuecomment-61367574.