1470815 - Security side panel is not visible for HTTP requests in the Console panel

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

The Network panel implements Security side panel displayed for HTTPS requests. However, this panel is not available in the Console panel when such HTTP request is expanded.

STR:

0) Make sure to enable 'Requests' filter in the Console panel
1) Select the Network panel
2) Load google.com (or any website with HTTPS request)
3) Select the first document request and see that the Security side-panel is available
4) Select the Console panel and expand the same request. The Security tab is not available -> BUG

I am using "Netmonitor" component, but "Console" component might also apply here.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Some info for anyone interested in this bug:

1) The visibility of the panel is calculated here:
https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/netmonitor/src/components/TabboxPanel.js#124

Note that the `request.securityState` is set to null in case of the Console panel and it's correctly set to `secure` in case of the Network panel.

2) Here is the place where the flag is received from the backend
https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/netmonitor/src/connector/firefox-data-provider.js#354

FirefoxDataProvider is responsible for fetching all HTTP data from the backend.

3) processNetworkUpdates is helper implementing additional processing. This might also be part of the issue.

https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/netmonitor/src/utils/request-utils.js#465

4) Here is the place where `TabboxPanel` (HTTP detail tabs) is rendered when HTTP request is expanded in the Console panel

https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/webconsole/components/message-types/NetworkEventMessage.js#142

5) Here is the place where NetworkDataProvider starts fetching data from the backend when HTTP request is expanded in the Console panel 

https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/webconsole/enhancers/net-provider.js#74

-

The `request.securityState` field is apparently lost somewhere on the way in case of the Console panel...

Honza

Comment 2

[theBugger]

Hi, I am new to open source environment. Can you help me in fixing this bug?

Comment 3

[Joan]

Hi,
    I'd like to work on this bug but its my first time. 
Thanks,
Cheryl.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to theBugger from comment #2)
> Hi, I am new to open source environment. Can you help me in fixing this bug?
Sorry for not replying sooner.
Are you still interested in this bug, should I assign it to you?

Also, if you use the "Need more information from" field (at the bottom of this page) you'll have good chance to get an answer from requested person much faster.

Honza

Comment 5

[Hemakshi Sachdev [:hemakshis]]

Hi Jan,
I would like to solve this bug as my first bug.
Would be helpful if you could guide me a little with the setup.

[I have understood the problem statement, of Security tab not being visible in the Console Panel for an HTTPS request while it is visible in the Network Panel for the same request.]

Thanks
Hemakshi

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Hemakshi Sachdev from comment #5)
> Hi Jan,
> I would like to solve this bug as my first bug.
Thanks for the interest!

I need to ask Cheryl first, but if there is no response within 2 days, ping me (need info me) and I'll assign it to you.

(In reply to Joan from comment #3)
> I'd like to work on this bug but its my first time. 
Still interested in this bug?

Honza

Comment 7

[Hemakshi Sachdev [:hemakshis]]

Hi Jan,
Any updates on this issue?

Thanks
Hemakshi

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Hemakshi Sachdev from comment #7)
> Hi Jan,
> Any updates on this issue?
Thanks for reaching out, assigned to you!

Let me know if you have any questions.

Honza

Comment 9

[Hemakshi Sachdev [:hemakshis]]

Hi
Thanks a lot!
Since this is my first time with open source, it would be helpful if you can guide me a little on how to begin with setup and everything.

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Hemakshi Sachdev from comment #9)
> Hi
> Thanks a lot!
> Since this is my first time with open source, it would be helpful if you can
> guide me a little on how to begin with setup and everything.

You might start reading here:
https://developer.mozilla.org/en-US/docs/Tools/Contributing
(how to start contributing to DevTools)

And this page is also useful:
https://firefox-dev.tools/

Honza

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I did some more analysis of this issue and here are some results:

(continue from #1)

> The `request.securityState` field is apparently lost somewhere on the way in case of the Console panel...

1) I think that the `securityState` is lost in `transformNetworkEventPacket` method:
https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/devtools/client/webconsole/utils/messages.js#260-273

We should make sure it's copied too like as follows:
  ...
  private: networkEvent.private,
  securityInfo: networkEvent.securityInfo,
  securityState: networkEvent.securityState,
});

This makes the "Security" tab visible.

2) The second problem is that when the "Security" tab is selected there is no content.
The tab only says "An error occurred <Not Available>"

This is because the security data was not fetched from the backend. The panel needs to sent "securityInfo" packet.

This should happen automatically in SecurityPanel.componentDidMount method using:
fetchNetworkUpdatePacket(connector.requestData, request, ["securityInfo"]);

Here:
https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/devtools/client/netmonitor/src/components/SecurityPanel.js#81

This method is indeed executed, but there is internal condition that checks whether the data has already been fetched from the backend (to avoid double fetch).


It's here:
https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/devtools/client/netmonitor/src/utils/request-utils.js#93

if (request[`${updateType}Available`] && !request[updateType]) {
  requestData(request.id, updateType);
}

The problem is that `request[updateType]` is set to "secure" - that's what we did in `transformNetworkEventPacket` method (step #1)

This is a collision, we are using the same field for two things:

Doing the following hack fixes the problem (just for testing purposes):
if (updateType == "securityInfo" && typeof(request.securityInfo) == "string") {
  request.securityInfo = null;
}

We need to map `secureInfo` to `secureState` - that's why we have `secureState` in the first place...

But doing the following (alternative for #1) doesn't work:

  ...
  private: networkEvent.private,
  securityState: networkEvent.securityInfo,
});

It's probably good idea to see why the Network panel works...


@Hemakshi: does that help to move forward?


Honza

Comment 12

[Hemakshi Sachdev [:hemakshis]]

(In reply to Jan Honza Odvarko [:Honza] from comment #11)
> I did some more analysis of this issue and here are some results:
> 
> (continue from #1)
> 
> > The `request.securityState` field is apparently lost somewhere on the way in case of the Console panel...
> 
> 1) I think that the `securityState` is lost in `transformNetworkEventPacket`
> method:
> https://searchfox.org/mozilla-central/rev/
> 246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/devtools/client/webconsole/utils/
> messages.js#260-273
> 
> We should make sure it's copied too like as follows:
>   ...
>   private: networkEvent.private,
>   securityInfo: networkEvent.securityInfo,
>   securityState: networkEvent.securityState,
> });
> 
> This makes the "Security" tab visible.

Yes, I agree with this point. In fact, just by adding `securityInfo: networkEvent.securityInfo`, makes the "Security" tab visible.
 
> @Hemakshi: does that help to move forward?

It indeed does! Thanks a lot for this. I had no clue about the collision happening in the `fetchNetworkUpdatePacket` method in the https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/devtools/client/netmonitor/src/utils/request-utils.js#93

> It's probably good idea to see why the Network panel works...

Yup, definitely will have to look at this.

Thanks a lot!

Hemakshi

Comment 13

[Hemakshi Sachdev [:hemakshis]]

(In reply to Jan Honza Odvarko [:Honza] from comment #11)

> It's probably good idea to see why the Network panel works...

For the Network Panel as far as I understand, all the requests are fetched from the back-end via firefox-data-provider and then stored in the redux store of Netmonitor, and from there used and passed on to the NetworkDetailsPanel component which in turn renders the TabboxPanel Component.

Also, with Console Panel, when we open a particular request, the `request` field provided to the TabboxPanel contains an extra key of `securityInfo: "secure"`, which is the main cause of collision (Refer comment #11). Whereas this key is not present in the `request` object provided to the Network Panel, and so works for it.

@Honza, please do let me know if I am wrong somewhere.

Hemakshi

Comment 14

[Hemakshi Sachdev [:hemakshis]]

Hi,
(In reply to Jan Honza Odvarko [:Honza] from comment #11)

> It's probably good idea to see why the Network panel works...

Update:

The Network Panel, never maintains a `securityInfo: "secure"` key in it's state, until and unless the Security Tab is not rendered { which on mounting calls fetchNetworkUpdatePacket(connector.requestData, request, ["securityInfo"]); }

The methods onNetworkEvent and onNetworkEventUpdate which are called by Console Panel in here https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/webconsole/enhancers/net-provider.js#75

Use the Console Panel's state to do the mapping of `securityState` in https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/netmonitor/src/connector/firefox-data-provider.js#354

So, if `networkInfo.securityInfo` is itself `null`, this sets `securityState` also to `null`, which then makes the value of Console Panel's `securityState` also to `null`. 
And so, the alternative

...
  private: networkEvent.private,
  securityState: networkEvent.securityInfo,
});

in https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/devtools/client/webconsole/utils/messages.js#255

also doesn't seems to work.

And using the method

  ...
  private: networkEvent.private,
  securityInfo: networkEvent.securityInfo,
  securityState: networkEvent.securityState,
});

leaves our state having a `securityInfo: "secure"` key, which causes the collision (Refer comment #11).


The Network Panel on the other hand does not use it's state data to call onNetworkEvent and onNetworkEventUpdate and so this problem not occurs for it. Right now, I don't how and from where these methods are being called for Network Panel.

I hope this helps in understanding the problem.

Again, if I am wrong somewhere please let me know

Hemakshi

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: security-panel.patch

@Hemakshi: I am attaching a patch that should fix the problem. Please try it out on your machine and let me know if it works for you.

Honza

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Also, I am changing the target component. It turned out that this is rather Console not Netmonitor bug.

It also needs a test. I think that good inspiration might be existing tests in the Console panel:
devtools/client/webconsole/test/mochitest/browser_webconsole_network*
(specifically 'browser_webconsole_network_messages_expand.js' might help)

Honza

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@hemakshi: I am seeing some test failures when pushing the patch to the Try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1041d82e14d7094b4d36fa87a229f1fd30a435e&selectedJob=194825926

Can you please look at that?

Use `mach` to run a test locally, e.g.:

`mach test devtools/client/netmonitor/test/browser_net_security-details.js`

Honza

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: security-panel.patch

@Hemakshi: I am attaching yet another patch that seems to fix the problem for me.

Here is try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=97ba1c761c4aaeb155bc83a8ad08eb38eb8fc6e0

Can you please try it on your machine?

Honza

Comment 19

[Hemakshi Sachdev [:hemakshis]]

(In reply to Jan Honza Odvarko [:Honza] from comment #18)
> Created attachment 9002757 [details] [diff] [review]
> security-panel.patch
> 
> @Hemakshi: I am attaching yet another patch that seems to fix the problem
> for me.
> 
> Here is try push:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=97ba1c761c4aaeb155bc83a8ad08eb38eb8fc6e0
> 
> Can you please try it on your machine?

Hi, yes it seems to work on machine too!

Hemakshi

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Assigning this to me, based on offline discussion with hemakshi.

@hemakshi: thanks for all the analysis and help!

Honza

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b39ce58378e1
Make sure the Security panel is visible for HTTP requests in the Console panel; r=nchevobbe

Comment 22

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b39ce58378e1