DD-31 dashboard anonymous accessible only with hash · Pull Request #56 · equilobe/DailyReportingTools

ghost · 2018-09-13T10:48:51Z

add hash to instance in order to access the dashboard report page with a special hash code, when not logged in
change default routes from RouteConfig
edit TimeZoneController action, because the id, which is a string in the timezone case, is mistaken as an action controller
change report angular controller, to allow the page to be loaded if the user is logged and the page to be loaded in anonymous mode if the user is not logged and only if the hash is correct for that specific report page

- add hash to instance in order to access the dashboard report page with a special hash code, when not logged in - change default routes from RouteConfig

Sebyd · 2018-09-14T08:27:54Z

DailyReportWeb/app/app.js

+                    report: function ($http, $location, $route) {
+                        var params = { instanceId: $route.current.params.instanceId, hash: $route.current.params.hash };
+
+                        $http.get('api/report/isDashboardAvailable', { params: params })


Do we need this? Can't we just have the page with a message instead? Like "No content yet" or something like that.

Yes, but before that I need to make sure the whole url is ok (instanceId and instance hash). Because I could just guess the instanceId, and just put a random string instead of hash. This method checks if the url is ok.

Why not put directly the hash as a query string param? So it needs to match that in order to get you to the dashboard.

I don't exactly understand. I still need to make a call to the server to check on that side if the hash is ok. The alternative would be to get the hash before and store it in the js, but that's just bad. Another alternative is to make the get of the dashboard with hash as well, and to throw an error which will come on js and check if it's the one related to the hash.

Are you talking about the last option?

yes, i think he is thinking about the last option. it's kind of weird to have an endpoint that checks if the dashboard is available. when you request the dashboard, if it's not available, just return there a boolean or something

Sebyd · 2018-09-14T08:38:18Z

DailyReportWeb/App_Start/RouteConfig.cs

                routeTemplate: "api/{controller}/{id}",
-                defaults: new { id = RouteParameter.Optional }
+                defaults: new { id = RouteParameter.Optional },
+                constraints: new { id = @"^$|\d+"}


Not sure this is the best solution. The id is not necessarily long. There can be string id's as well in the future.

I came across this problem, but I found no other way to make it generic. If the id should be a string, then the method should be named exactly and the parameter received with FromUri.

Where does this problem occur? So that I can have a look at the controller

App.js, Timezone. I changed in this pull request as well.

The instances controller works the same way. There is one method with Get() and one with Get(id). And they worked both.
The mistake that you do is that you pass the name of the method and it is considered to be the id. If you want the get the resources you do: http.get(api/controller) -> this gets the list. http.get(api/controller/id) -> gets the element. Try this way without the routes changed. It should work.

Ok, and what happens if I want multiple parameters? Also, there is one hardcoded route for the account controller, so making a generic change for the difference between the id and action seems like a good thing to do.

- update logic to verify the validity of the url in the request for getting the dashboard, instead of making another pre-request

- update the api url - update the conditions for dashboard availability

- restore routes changes - update the angular call for getting the dashboard - add parameter to get the unavailable page for dashboard

- restore some changes

- restore unwanted change

- refactor code

Sebyd · 2018-09-17T14:40:09Z

DailyReportWeb/app/report.js

                    ctrl.isLoading = true;

-                    $http.get("/api/report/" + ctrl.instanceId)
+                    $http.get("/api/report/?instanceId=" + ctrl.instanceId + "&hash=" + ctrl.hash + "&isAuthenticated=" + isAuth)


remove the "/" here. it should be "report?instanceId="

Sebyd · 2018-09-17T14:42:15Z

Equilobe.DailyReport.Models/Dashboard/DashboardData.cs

+        public List<DashboardItem> Items { get; set; }
+
+
+        public static DashboardData Unavailable


make this a method instead of a property. not all objects of type DashboardData have an invalid DashboardData property on them.
Instead, just make this as a method that returns the same thing. Also keep the name.

- refactor

DD-31

7a4c5ab

- add hash to instance in order to access the dashboard report page with a special hash code, when not logged in - change default routes from RouteConfig

ghost changed the title ~~DD-31~~ DD-31 dashboard anonymous accessible only with hash Sep 13, 2018

ghost requested a review from Sebyd September 13, 2018 10:49

Sebyd reviewed Sep 14, 2018

View reviewed changes

Georgian Tanase added 7 commits September 14, 2018 19:53

Merge branch 'dashboard' into DD-31-dashboard-hash

af01814

DD-31

fe04954

- update logic to verify the validity of the url in the request for getting the dashboard, instead of making another pre-request

DD-31

355de4a

- update the api url - update the conditions for dashboard availability

DD-31

75f74e5

- restore routes changes - update the angular call for getting the dashboard - add parameter to get the unavailable page for dashboard

DD-31

28de773

- restore some changes

DD-31

7a9e110

- restore unwanted change

DD-31

c6787d1

- refactor code

Sebyd requested changes Sep 17, 2018

View reviewed changes

DD-31

3a1cb84

- refactor

Sebyd approved these changes Sep 18, 2018

View reviewed changes

ghost merged commit 286fea0 into dashboard Sep 18, 2018

ghost deleted the DD-31-dashboard-hash branch September 18, 2018 07:50

This pull request was closed.

		public List<DashboardItem> Items { get; set; }


		public static DashboardData Unavailable

Conversation

ghost commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sebyd Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sebyd Sep 14, 2018 •

edited

Loading